This is an automated email from the ASF dual-hosted git repository. chrisdutz pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit 3e72f96c5ad080191975ebe1297c5fbc85831b96 Author: Christofer Dutz <[email protected]> AuthorDate: Sat Aug 15 14:06:06 2026 +0200 fix(plc4j/bacnet): Close the reader an EDE file is parsed from parseFileDatapoints handed a FileReader to commons-csv and closed neither it nor the CSVParser on any path, so parsing an EDE file leaked a file handle that was never released. parseDirectory calls it once per file, so pointing the driver at a directory of EDE files leaks a handle per file and, on Windows, leaves every one of them locked. Wrap both in try-with-resources. This is the same defect that was just fixed in the OPC UA key store loading; a sweep over the remaining main sources found no others. The TLS transport and the ctrlx API client factory already close their streams correctly. Adds EdeParserTest, the first test in this module. It covers the leak from two sides, because neither check works everywhere: deleting the file after parsing catches it on Windows, and counting open descriptors over 100 parses catches it on POSIX. Reverting the fix makes the descriptor test report exactly 100 leaked handles. --- .../apache/plc4x/java/bacnetip/ede/EdeParser.java | 5 +- .../plc4x/java/bacnetip/ede/EdeParserTest.java | 121 +++++++++++++++++++++ 2 files changed, 123 insertions(+), 3 deletions(-) diff --git a/plc4j/drivers/bacnet/src/main/java/org/apache/plc4x/java/bacnetip/ede/EdeParser.java b/plc4j/drivers/bacnet/src/main/java/org/apache/plc4x/java/bacnetip/ede/EdeParser.java index 9df9f62336..f9c7c25eeb 100644 --- a/plc4j/drivers/bacnet/src/main/java/org/apache/plc4x/java/bacnetip/ede/EdeParser.java +++ b/plc4j/drivers/bacnet/src/main/java/org/apache/plc4x/java/bacnetip/ede/EdeParser.java @@ -88,9 +88,8 @@ public class EdeParser { } private Map<BacNetIpTag, Datapoint> parseFileDatapoints(File edeFile) { - try { - Reader in = new FileReader(edeFile); - final CSVParser parser = CSVFormat.newFormat(';').parse(in); + try (Reader in = new FileReader(edeFile); + CSVParser parser = CSVFormat.newFormat(';').parse(in)) { final Iterator<CSVRecord> iterator = parser.iterator(); // Skip the header. diff --git a/plc4j/drivers/bacnet/src/test/java/org/apache/plc4x/java/bacnetip/ede/EdeParserTest.java b/plc4j/drivers/bacnet/src/test/java/org/apache/plc4x/java/bacnetip/ede/EdeParserTest.java new file mode 100644 index 0000000000..66d4611842 --- /dev/null +++ b/plc4j/drivers/bacnet/src/test/java/org/apache/plc4x/java/bacnetip/ede/EdeParserTest.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.plc4x.java.bacnetip.ede; + +import com.sun.management.UnixOperatingSystemMXBean; +import org.apache.plc4x.java.bacnetip.ede.model.Datapoint; +import org.apache.plc4x.java.bacnetip.ede.model.EdeModel; +import org.apache.plc4x.java.bacnetip.tag.BacNetIpTag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.lang.management.ManagementFactory; +import java.lang.management.OperatingSystemMXBean; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * The parser used to hand its {@link java.io.FileReader} to commons-csv and never close either one, + * so every parsed EDE file leaked a file handle for the lifetime of the JVM. On Windows the leaked + * handle also keeps the file locked, which is how it first surfaced. + */ +class EdeParserTest { + + @TempDir + Path tempDir; + + @Test + void parsesADatapoint() throws Exception { + File edeFile = writeEdeFile("edeDataText.csv"); + + EdeModel model = new EdeParser().parseFile(edeFile); + + Datapoint datapoint = model.getDatapoint(new BacNetIpTag(12L, 2, 7)); + assertNotNull(datapoint, "the single data row is picked up"); + assertEquals("Temperature", datapoint.getObjectName()); + } + + /** + * Windows refuses to delete a file while a handle on it is open, so this fails there if the + * reader is left open. On POSIX the delete succeeds either way - {@link #doesNotLeakFileHandles} + * is what catches the leak on those platforms. + */ + @Test + void releasesTheFileAfterParsing() throws Exception { + File edeFile = writeEdeFile("edeDataText.csv"); + + new EdeParser().parseFile(edeFile); + + Files.delete(edeFile.toPath()); + assertFalse(edeFile.exists()); + } + + /** + * The delete check above is a no-op on POSIX, so count the descriptors instead. + */ + @Test + void doesNotLeakFileHandles() throws Exception { + OperatingSystemMXBean osBean = ManagementFactory.getOperatingSystemMXBean(); + assumeTrue(osBean instanceof UnixOperatingSystemMXBean, "open descriptors are not countable here"); + UnixOperatingSystemMXBean unixBean = (UnixOperatingSystemMXBean) osBean; + + File edeFile = writeEdeFile("edeDataText.csv"); + EdeParser parser = new EdeParser(); + // One parse up front so that anything opened lazily on the first pass is not counted. + parser.parseFile(edeFile); + + long before = unixBean.getOpenFileDescriptorCount(); + for (int i = 0; i < 100; i++) { + parser.parseFile(edeFile); + } + long leaked = unixBean.getOpenFileDescriptorCount() - before; + + // Unrelated activity can open a descriptor or two, but not a hundred of them. + assertTrue(leaked < 10, "leaked " + leaked + " file descriptors over 100 parses"); + } + + /** + * Five rows of preamble, the layout version in the second column, one more row to skip and the + * column names - see {@link EdeParser} and the version 2 layout. + */ + private File writeEdeFile(String name) throws Exception { + Path file = tempDir.resolve(name); + Files.writeString(file, String.join("\n", + "#Data Exchange File", + "PROJECT_NAME;test", + "VERSION_OF_REFERENCEFILE;1", + "TIMESTAMP_OF_LAST_CHANGE;2026-01-01", + "AUTHOR_OF_LAST_CHANGE;plc4x", + "VERSION_OF_LAYOUT;2", + "#mandatory;mandatory;mandatory;mandatory;mandatory", + "keyname;device obj.-instance;object-name;object-type;object-instance;description;" + + "default value;min. value;max. value;commandable;supports COV;hi limit;lo limit;" + + "state-text-reference;unit-code;vendor-specific-address", + "temp-1;12;Temperature;2;7;Outside temperature;0;-50;150;N;Y;100;-40;;62;" + ) + "\n"); + return file.toFile(); + } +}
