thanks for your feedback: good to have diverse points of view (and my intent is not to avoid that diversity)
TL;DR please VOTE in a vote thread = +1 or even +0 if you don't feel confident enough, and remember that we are looking for strong reasons not to release once you clearly vote, yes, adding some ideas of future improvements is ok (now that you voted, this intent is clear) more in depth: yes, this is recurring = why I wrote once to try to improve the feedback loop, that once improved, will be much more efficient my point is that having a vote as part of the feedback would feel like working with the release manager on what we are currently doing = decide if nothing really strong happens against the release. As a regular release manager, I often feel that the vote is more done now as non-actionable "BTW, I'm not happy, there are issues" approach, which makes this release manager role harder than what it should be. in addition, one bad feeling I have with all these AI generated feedbacks is that "nobody has any problem, but my tool says things are wrong": but opening that discussion may hijack this vote thread even more that why I'm asking for the TL;DR: please VOTE first in vote threads, which will make secondary comments more actionable this clarification is useful, again, thanks David for asking for it On 2026/06/30 21:18:16 David Jencks wrote: > I don’t actually work on maven, so feel free to discount my opinion. I don’t > really understand your point of view. I doubt Elliotte has an infinite amount > of time to work on maven, so I think he takes vote threads as a trigger to > look around for possible problems and improvements in the release candidate > and generally asks, “are any of these important enough to redo the release?” > While this might seem annoying or distracting, upon reflection I think this > is a valuable contribution. While I think this is the first time he’s used AI > to look for problems, I think it’s conceptually similar to many of his past > posts on vote threads. > > David Jencks > Sent from my iPhone > > > On Jun 30, 2026, at 1:33 PM, Hervé Boutemy <[email protected]> wrote: > > > > one day, you'll have to learn what voting means: such work is useful for > > next release, not in a vote thread > > > >> On 2026/06/30 13:00:20 Elliotte Rusty Harold wrote: > >> Another grab bag of bugs the LLM found. At first glance, none of these > >> look too serious though: > >> > >> # Bug Analysis: maven-resolver > >> > >> Generated by code analysis of `/Users/elharo/maven-resolver` > >> (v2.0.21-SNAPSHOT). > >> > >> --- > >> > >> ## HIGH Severity > >> > >> ### 1. NPE — `getClassLoader().getResource()` returns null > >> **File:** > >> `maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java:265` > >> > >> ```java > >> String url = clazz.getClassLoader().getResource(className).toString(); > >> ``` > >> > >> `ClassLoader.getResource()` returns `null` when the resource is not > >> found, and `getClassLoader()` returns `null` for classes loaded by the > >> bootstrap class loader. Either condition causes an NPE on > >> `.toString()`. The method `getJarPath()` is called during client > >> initialization to build the classpath for forking a child process. > >> > >> ### 2. NPE — `receive()` reads volatile `input` without synchronization > >> **File:** > >> `maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java:288-311` > >> > >> ```java > >> void receive() { > >> try { > >> while (true) { > >> int id = input.readInt(); // NPE here if input null > >> ``` > >> > >> `input` is a volatile field (line 86). `receive()` reads it without > >> holding the `IpcClient` monitor. Meanwhile, `close(Throwable)` (line > >> 351, `synchronized`) sets `input = null` (line 360). Between the > >> volatile read at line 291 and the `input.readInt()` call, another > >> thread can call `close()`, nulling `input`. The same pattern applies > >> to `send()` at line 316 with `output`. > >> > >> ### 3. NPE — `getAddress()` can NPE on `socket.getLocalAddress()` > >> **File:** > >> `maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java:447-453` > >> > >> ```java > >> private String getAddress() { > >> try { > >> return SocketFamily.toString(socket.getLocalAddress()); > >> } catch (IOException e) { > >> return "[not bound]"; > >> } > >> } > >> ``` > >> > >> `socket` is volatile (line 84) and set to `null` in `close()` (line > >> 359). If `close()` runs concurrently with `toString()` (which calls > >> `getAddress()`), `socket` is null and `socket.getLocalAddress()` > >> throws NPE, which is **not** an `IOException` and is thus uncaught. > >> > >> ### 4. Resource leak — `BufferedReader` never closed in > >> `parseMultiResource()` > >> **File:** > >> `maven-resolver-test-util/src/main/java/org/eclipse/aether/internal/test/util/DependencyGraphParser.java:161` > >> > >> ```java > >> BufferedReader reader = new BufferedReader(new > >> InputStreamReader(res.openStream(), StandardCharsets.UTF_8)); > >> ``` > >> > >> The `BufferedReader` (and its underlying `InputStream`) is **never > >> closed**. If `parse(reader)` throws an IOException, the stream leaks. > >> Compare with the correctly-implemented `parse(URL)` method at line > >> 174-188 which uses try-finally. > >> > >> --- > >> > >> ## MEDIUM Severity > >> > >> ### 5. Catching `Throwable` in non-framework code > >> **File:** > >> `maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/PutTaskRequestContent.java:246, > >> 298` > >> > >> ```java > >> } catch (Throwable t) { > >> lockedSetTerminal(Content.Chunk.from(t, true)); > >> } > >> ``` > >> > >> Catches `OutOfMemoryError`, `StackOverflowError`, `InternalError`, > >> etc. These should propagate. Wrapping them as terminal chunks converts > >> fatal JVM errors into regular failure conditions, making them > >> invisible to diagnostics. > >> > >> ### 6. Production error logging to `System.out` > >> **File:** > >> `maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcServer.java:196` > >> > >> ```java > >> private static void error(String msg, Throwable t) { > >> System.out.println("[ipc] [error] " + msg); > >> t.printStackTrace(System.out); > >> } > >> ``` > >> > >> All errors in the IPC server go to stdout instead of a proper logging > >> framework (SLF4J). In a production Maven build, these messages are > >> invisible or interleaved with build output. Same issue for `debug()` > >> (line 184) and `info()` (line 190) in the same file. > >> > >> ### 7. Listener RuntimeException silently swallowed by default > >> **Files:** > >> - > >> `maven-resolver-util/src/main/java/org/eclipse/aether/util/listener/ChainedRepositoryListener.java:117` > >> - > >> `maven-resolver-util/src/main/java/org/eclipse/aether/util/listener/ChainedTransferListener.java:118` > >> > >> ```java > >> @SuppressWarnings("EmptyMethod") > >> protected void handleError(...) { > >> // default just swallows errors > >> } > >> ``` > >> > >> All `RuntimeException`s thrown by any chained listener (NPEs, > >> `IndexOutOfBoundsException`, etc.) are silently swallowed unless the > >> user subclasses and overrides `handleError`. This makes listener bugs > >> invisible. > >> > >> ### 8. Volatile fields with inconsistent synchronization > >> **File:** > >> `maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java:78-87` > >> > >> Fields `initialized`, `socket`, `output`, `input`, `receiver` are > >> `volatile` but their compound use (read + method call) lacks > >> synchronization. `send()` reads `output` into a local variable at line > >> 316 without holding the monitor, then synchronizes on the local > >> reference at line 323, but `close()` nulls `output` under > >> `synchronized(this)` at line 361. `receive()` has the same pattern > >> with `input`. > >> > >> ### 9. SyncContext double-close risk > >> **Files:** > >> - > >> `maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java:189-190, > >> 393-397, 430-432` > >> - > >> `maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultMetadataResolver.java:142-143, > >> 302-373` > >> > >> Two `SyncContext` instances (`shared` and `exclusive`) are created in > >> try-with-resources. Inside `resolve()`, `current` starts as `shared`, > >> and when switching to `exclusive` (line 393-396), `current.close()` is > >> called, then `current = exclusive`. The outer try-with-resources also > >> closes both on exit (line 202 for try-with-resources, and line 431 for > >> the inner `current.close()`). While `SyncContext.close()` is > >> documented as idempotent, any implementation that violates this > >> contract would cause issues. > >> > >> ### 10. InterruptedException causes task to be silently dropped > >> **File:** > >> `maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/SmartExecutor.java:177-179` > >> > >> ```java > >> } catch (InterruptedException e) { > >> Thread.currentThread().interrupt(); > >> } > >> ``` > >> > >> In `Limited.submit(Runnable)` (line 155), if `semaphore.acquire()` > >> throws `InterruptedException`, the interrupt flag is restored but the > >> caller's task is never executed. The method simply returns. The > >> `submit(Callable)` variant (line 183-213) correctly handles this by > >> returning a failed `CompletableFuture`. The `Runnable` overload should > >> do something analogous (e.g., run the task inline or throw). > >> > >> --- > >> > >> ## LOW Severity > >> > >> ### 11. Minor resource leak in `parseLiteral()` > >> **File:** > >> `maven-resolver-test-util/src/main/java/org/eclipse/aether/internal/test/util/DependencyGraphParser.java:134-138` > >> > >> ```java > >> BufferedReader reader = new BufferedReader(new > >> StringReader(dependencyGraph)); > >> DependencyNode node = parse(reader); > >> reader.close(); // never reached if parse() throws > >> ``` > >> > >> While `StringReader.close()` is a no-op, the pattern is inconsistent > >> with the rest of the codebase and represents a latent bug if the > >> implementation changes. > >> > >> ### 12. Empty catch swallows exceptions in `tryUnlock()` > >> **File:** > >> `maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcNamedLock.java:108-115` > >> > >> ```java > >> private void tryUnlock(String contextId) { > >> try { > >> client.unlock(contextId); > >> } catch (Exception e) { > >> // Best-effort cleanup > >> } > >> } > >> ``` > >> > >> All exceptions from `unlock()` during timeout cleanup are silently > >> discarded. While documented as intentional, this can mask real > >> IO/connectivity issues. > >> > >> ### 13. Non-thread-safe access in synchronized `Results` class > >> **File:** > >> `maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegate.java:571-598` > >> > >> `addException()` and `addCycle()` are `synchronized` on `Results`, but > >> they call `result.getExceptions()` and `result.getCycles()` on a > >> `CollectResult` that may not be thread-safe. If `CollectResult`'s > >> internal collections are not synchronized or safely published, > >> concurrent modifications may not be visible. > >> > >> ### 14. `putIfAbsent()` fast-path race > >> **File:** > >> `maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/ConcurrentWeakCache.java:124-146` > >> > >> ```java > >> V existing = get(key); // fast path > >> if (existing != null) { > >> return existing; > >> } > >> ``` > >> > >> The fast-path `get()` uses a ThreadLocal `LookupKey`. Between `get()` > >> returning null and `merge()` at line 135, another thread can insert a > >> new value. The merge correctly handles this, but if `get()` returns a > >> non-null reference to a key that gets GC'd immediately after, we > >> return a stale reference. This is a correct-by-accident pattern — the > >> merge would fix it, but we return early. > >> > >> ### 15. Pre-Java-7 stream handling in test utilities > >> **Files:** > >> - > >> `maven-resolver-test-util/src/main/java/org/eclipse/aether/internal/test/util/TestFileUtils.java:172-325` > >> - > >> `maven-resolver-test-util/src/main/java/org/eclipse/aether/internal/test/util/TestFileProcessor.java:63-151` > >> > >> Methods manually close streams in try-catch-finally blocks instead of > >> using try-with-resources. If an explicit `close()` call throws before > >> the finally block, open resources are leaked. Functionally correct in > >> happy-path but fragile. > >> > >> ### 16. `printStackTrace()` used instead of logging in tests > >> Multiple test files use `e.printStackTrace()` instead of proper > >> assertions or test framework logging, making test failures harder to > >> diagnose. Affected files include: > >> - > >> `maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultArtifactResolverTest.java:740, > >> 752, 805, 874, 975, 987` > >> - `maven-resolver-demos/src/main/java/.../Booter.java:111` > >> - > >> `maven-resolver-api/src/test/java/org/eclipse/aether/DefaultSessionDataTest.java:118` > >> - > >> `maven-resolver-api/src/test/java/org/eclipse/aether/DefaultRepositoryCacheTest.java:82` > >> > >> --- > >> > >> ## Summary > >> > >> | Severity | Count | Key Areas | > >> |----------|-------|-----------| > >> | HIGH | 4 | IpcClient (3 NPEs), DependencyGraphParser (resource > >> leak) | > >> | MEDIUM | 6 | PutTaskRequestContent (Throwable catch), > >> IpcServer (stdout logging), ChainedListener (silent swallow), > >> IpcClient (sync), SyncContext double-close, SmartExecutor (dropped > >> task) | > >> | LOW | 6 | Minor resource leak, swallowed exception, thread > >> safety in Results, ConcurrentWeakCache race, old stream handling, > >> printStackTrace | > >> > >> The **IPC named-locks module** (`maven-resolver-named-locks-ipc`) > >> accounts for the majority of HIGH-severity issues — it has multiple > >> NPEs from unsynchronized volatile field access. > >> > >>> On Tue, Jun 30, 2026 at 8:09 AM <[email protected]> wrote: > >>> > >>> Hello Maven, > >>> > >>> I'd like to call a vote on releasing the following artifacts as > >>> Apache Maven Resolver 2.0.20. This vote is being conducted using an > >>> Alpha version of the Apache Trusted Releases (ATR) platform. > >>> Please report any bugs or issues to the ASF Tooling team. > >>> > >>> The release candidate page, including downloads, can be found at: > >>> > >>> https://release-test.apache.org/vote/maven-resolver/2.0.20 > >>> > >>> The release artifacts are signed with one or more OpenPGP keys from: > >>> > >>> https://dist.apache.org/repos/dist/release/maven/KEYS > >>> > >>> Maven staging repository: > >>> > >>> https://repository.apache.org/content/repositories/maven-2440/ > >>> > >>> Release notes (draft): > >>> > >>> https://gist.github.com/cstamas/1a20ff11105992bf90a982cbae34036e > >>> > >>> Changes since the last release: > >>> > >>> > >>> https://github.com/apache/maven-resolver/compare/maven-resolver-2.0.18...maven-resolver-2.0.20 > >>> > >>> Staging site: > >>> > >>> https://maven.apache.org/resolver-archives/resolver-LATEST/ > >>> > >>> Site deployed to SVN; sync pending; SVN site source: > >>> > >>> > >>> https://svn.apache.org/repos/asf/maven/website/components/resolver-archives/resolver-LATEST/index.html > >>> > >>> Please review the release candidate and vote accordingly. > >>> > >>> [ ] +1 Release this package > >>> [ ] +0 Abstain > >>> [ ] -1 Do not release this package (please provide specific comments) > >>> > >>> You can vote on ATR at the URL above, or manually by replying to this > >>> email. > >>> > >>> The vote is open for 72 hours. > >>> > >>> > >>> Thanks, > >>> Tamas Cservenak (cstamas) > >>> > >>> This email was sent by [email protected] on the Apache Trusted Releases > >>> platform > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe, e-mail: [email protected] > >>> For additional commands, e-mail: [email protected] > >>> > >> > >> > >> -- > >> Elliotte Rusty Harold > >> [email protected] > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [email protected] > >> For additional commands, e-mail: [email protected] > >> > >> > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
