+1 (nb) On Wed, Jul 1, 2026 at 8:28 AM Hervé Boutemy <[email protected]> wrote:
> 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] > >
