Skip to content

SessionLoop.shutdown check-then-act is not atomic; cleanup runs twice on concurrent close #50

@nficano

Description

@nficano

The shutdown method in arcp-runtime/src/main/java/dev/arcp/runtime/session/SessionLoop.java around line 162 reads phase on line 163, then writes phase = Phase.CLOSED on line 166 as two separate operations on a volatile field. Concurrent callers — and there are at least three: onError from the inbound subscription, onComplete from the inbound subscription, and the heartbeat tick deciding HEARTBEAT_LOST — can both observe phase != CLOSED, both set it to CLOSED, and both run the rest of the cleanup body. The cleanup body is not idempotent: credentialBinding.revokeAll(rec) runs the configured CredentialProvisioner.revoke for each issued credential, so a second pass invokes the provider's external revoke API twice; runtime.removeSession(this) is called twice; the heartbeat ScheduledFuture and per-job watchdog ScheduledFutures are cancel(false)'d twice. The downstream provider API may not be idempotent, and double-cancel of a future is benign, but the duplicate provisioner call is not.\n\nFix prompt: In arcp-runtime/src/main/java/dev/arcp/runtime/session/SessionLoop.java change the volatile Phase phase field to an AtomicReference<Phase> phase initialized to AWAITING_HELLO. In shutdown, use if (!phase.getAndSet(Phase.CLOSED).equals(Phase.CLOSED)) return — actually use the negation: if (phase.getAndSet(Phase.CLOSED) == Phase.CLOSED) return; so only the first caller proceeds with cleanup. Update every other read of phase in the class (handle around line 198, the JobContext.cancelled implementations, send around line 812) to use phase.get(). Update phase assignments in doHandshake (lines 249) to use phase.set(Phase.ACTIVE). Add a JUnit test that fires shutdown("a") and shutdown("b") concurrently against a SessionLoop with one active job and a recording CredentialProvisioner, and asserts the provisioner observes exactly one revoke call per issued credential.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingseverity:mediumMedium severity

    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