Skip to content

LLM grabbag #1944

Description

@elharo

Affected version

HEAD

Bug description

These were all pulled out by throwing the code at an LLM. This issue lists multiple independent bugs. If anyone wants to work on any of these, note it here and open a new issue just for that bug. Be careful that some of these are likely false reports, though closing those is also useful work.

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

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

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

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

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

} 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

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
@SuppressWarnings("EmptyMethod")
protected void handleError(...) {
    // default just swallows errors
}

All RuntimeExceptions 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

} 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

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

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

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions