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]
