Skip to content

[Bug] Server cleanup callbacks dereference null when connection futures complete exceptionally or are cancelled #79

Description

@shaaravraghu

Issue type
Correctness bug

Severity
High

Priority
P1

Affected modules
base-network

Affected files
base-network/src/main/java/build/base/network/Server.java

Verification status
Source inspection

Problem summary
Server registers cleanup callbacks that assume a non-null Connection, but CompletableFuture.whenComplete can invoke those callbacks with connection == null when the future is cancelled or completed exceptionally.

Evidence
At line 146, connection.onClosed().whenComplete((c, t) -> this.connections.remove(c.getRemoteIdentity())) dereferences c without checking t.
At lines 163-166, the finally block cancels futures and then runs future.whenComplete((connection, throwable) -> connection.close()), which dereferences connection even for cancelled futures.

Relevant references
base-network/src/main/java/build/base/network/Server.java:146
base-network/src/main/java/build/base/network/Server.java:163
base-network/src/main/java/build/base/network/Server.java:166

Why this matters
Cleanup code should be the safest part of the lifecycle.
A null dereference during shutdown or exceptional completion can obscure the original failure and leave connection bookkeeping inconsistent.

Reproduction steps

  1. Inspect Server.start().
  2. Trace the finally path where pending connection futures are cancelled.
  3. Note that cancelled and exceptional CompletableFuture callbacks receive null values and are still dereferenced.

Expected result
Cleanup callbacks should tolerate cancelled and exceptionally completed futures without dereferencing a null Connection.

Actual result
Both callback sites assume successful completion and can throw NullPointerException.

Suggested fix direction
Guard on throwable == null && connection != null before using the connection object.
For the onClosed() callback, only remove the tracked entry when a real connection instance is available.

Acceptance criteria
Server shutdown and exceptional completion paths do not throw NullPointerException.
Tests cover cancelled, exceptionally completed, and normally completed connection futures.

Suggested labels
bug
network
lifecycle

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    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