You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2026/09/01 10:29:54 UTC
(maven-resolver) branch master updated: Optimize TrackingFileManager unit tests and fix potential lock leaks (#2094) (#2102)
This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 323ce6e11 Optimize TrackingFileManager unit tests and fix potential lock leaks (#2094) (#2102)
323ce6e11 is described below
commit 323ce6e111331f60e7e0e80c6ad5322c7a4489af
Author: Sylwester Lachiewicz <sl...@apache.org>
AuthorDate: Tue Sep 1 12:29:46 2026 +0200
Optimize TrackingFileManager unit tests and fix potential lock leaks (#2094) (#2102)
### Summary
1. **Prevent File Descriptor Exhaustion in Tests**: Reduces iteration count in `TrackingFileManagerTestSupport` from 1,000 to 100 in leak checks and canonical path locking tests.
2. **Lock Leak Hardening**:
- `NamedLockFactoryAdapter`: Ensures `namedLock.close()` is invoked if lock acquisition fails/times out or is interrupted, preventing a permanent +1 reference count leak.
- `CompositeNamedLock`: Automatically unlocks previously acquired sub-locks if a thread is interrupted or encounters an exception mid-way during multi-lock acquisition.
### Root Cause Analysis (macOS / CI FD Exhaustion)
* In `TrackingFileManagerTestSupport`, each loop iteration creates a new temporary file with a distinct path, creating a distinct `NamedLockKey` and lock file.
* `FileLockNamedLockFactory` maintains an LRU pool of up to `MAX_CACHED_CHANNELS = 200` open file channels for closed locks.
* Running 1,000 iterations per test method saturates the 200-channel cache. On macOS (where default `ulimit -n` is 256), combining 200 cached channels with JVM/Surefire baseline descriptors (~50 FDs) directly exhausts the 256 FD limit and throws `Too many open files`.
* Lowering iterations to 100 ensures channel cache growth peaks at ~100 FDs per test method (staying safely below 256), while still thoroughly verifying leak-free behavior and canonical path deduplication.
* Reduces test runtime from 18.8s to 5.0s (-73% speedup).
Resolves #2094; relates to #2096.
---
.../synccontext/named/NamedLockFactoryAdapter.java | 12 +++++-
.../impl/TrackingFileManagerTestSupport.java | 8 ++--
.../aether/named/support/CompositeNamedLock.java | 49 ++++++++++++----------
3 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java
index 81d11768c..acc0c8c3b 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java
@@ -258,13 +258,21 @@ public final class NamedLockFactoryAdapter {
}
} catch (InterruptedException e) {
// if we are here, means we were interrupted: fail
- close();
+ try {
+ namedLock.close();
+ } finally {
+ close();
+ }
Thread.currentThread().interrupt();
throw new RuntimeException(e);
}
}
// if we are here, means all attempts were unsuccessful: fail
- close();
+ try {
+ namedLock.close();
+ } finally {
+ close();
+ }
String message = "Could not acquire " + lockKind + " lock for "
+ lockSubjects(artifacts, metadatas) + " in " + timeStr
+ "; consider using '" + CONFIG_PROP_TIME
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTestSupport.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTestSupport.java
index a72f52cf6..214e4140d 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTestSupport.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTestSupport.java
@@ -120,7 +120,7 @@ public abstract class TrackingFileManagerTestSupport {
void testReadNoFileLeak(FS fs) throws Exception {
TrackingFileManager tfm = createTrackingFileManager(fs);
- for (int i = 0; i < 1000; i++) {
+ for (int i = 0; i < 100; i++) {
Path propFile = createTmpFile(fs);
assertNotNull(tfm.read(propFile));
assertDoesNotThrow(() -> Files.delete(propFile), "Leaked file" + propFile);
@@ -157,7 +157,7 @@ public abstract class TrackingFileManagerTestSupport {
Map<String, String> updates = new HashMap<>();
updates.put("k", "v");
- for (int i = 0; i < 1000; i++) {
+ for (int i = 0; i < 100; i++) {
Path propFile = createTmpFile(fs);
assertNotNull(tfm.update(propFile, updates));
assertDoesNotThrow(() -> Files.delete(propFile), "Leaked file" + propFile);
@@ -169,7 +169,7 @@ public abstract class TrackingFileManagerTestSupport {
public void testDeleteFileIsGone(FS fs) throws Exception {
TrackingFileManager tfm = createTrackingFileManager(fs);
- for (int i = 0; i < 1000; i++) {
+ for (int i = 0; i < 100; i++) {
Path propFile = createTmpFile(fs);
assertTrue(tfm.delete(propFile));
assertFalse(Files.exists(propFile), "File is not gone");
@@ -196,7 +196,7 @@ public abstract class TrackingFileManagerTestSupport {
threads[i] = new Thread(() -> {
try {
- for (int i1 = 0; i1 < 1000; i1++) {
+ for (int i1 = 0; i1 < 100; i1++) {
assertNotNull(tfm.read(file));
HashMap<String, String> update = new HashMap<>();
update.put("wasHere", Thread.currentThread().getName() + "-" + i1);
diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/CompositeNamedLock.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/CompositeNamedLock.java
index 583d19583..f4a1e7058 100644
--- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/CompositeNamedLock.java
+++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/CompositeNamedLock.java
@@ -66,29 +66,34 @@ public final class CompositeNamedLock extends NamedLockSupport {
final String lockKind = shared ? "shared" : "exclusive";
LOGGER.trace(
"{}: Need {} {} lock(s) of {} in {}", key().name(), locks.size(), lockKind, key().resources(), timeStr);
- for (NamedLock namedLock : locks.values()) {
- LOGGER.trace("{}: Acquiring {} lock for '{}'", key().name(), lockKind, namedLock.key());
-
- boolean locked;
- if (shared) {
- locked = namedLock.lockShared(time, timeUnit);
- } else {
- locked = namedLock.lockExclusively(time, timeUnit);
- }
-
- if (!locked) {
- LOGGER.trace(
- "{}: Failed to acquire {} lock for '{}' in {}",
- key().name(),
- lockKind,
- namedLock.key(),
- timeStr);
-
- unlockAll(step);
- return false;
- } else {
- step.push(namedLock);
+ try {
+ for (NamedLock namedLock : locks.values()) {
+ LOGGER.trace("{}: Acquiring {} lock for '{}'", key().name(), lockKind, namedLock.key());
+
+ boolean locked;
+ if (shared) {
+ locked = namedLock.lockShared(time, timeUnit);
+ } else {
+ locked = namedLock.lockExclusively(time, timeUnit);
+ }
+
+ if (!locked) {
+ LOGGER.trace(
+ "{}: Failed to acquire {} lock for '{}' in {}",
+ key().name(),
+ lockKind,
+ namedLock.key(),
+ timeStr);
+
+ unlockAll(step);
+ return false;
+ } else {
+ step.push(namedLock);
+ }
}
+ } catch (Throwable t) {
+ unlockAll(step);
+ throw t;
}
steps.push(step);
return true;