zeweihan commented on issue #1993:
URL: 
https://github.com/apache/maven-resolver/issues/1993#issuecomment-5032690714

   This is the canonical "accepted ≠ stored" failure class — and for a 
dependency-resolution tool it's arguably one of the worst places it can occur.
   
   `submit(Runnable)` is a fire-and-forget API whose entire contract is *"I 
take your task; it will eventually run."* There's no return value to inspect, 
so the caller's only basis for trust is that the method returned normally. The 
three dispositions that honor that contract are: (1) the task runs, (2) the 
task is enqueued for later execution, or (3) the caller is told it did not — 
via exception or an exceptionally-completed future. The current code adds a 
fourth, invalid state: *accepted, never enqueued, no signal*. That isn't a 
silent failure — it's silent non-execution, and unlike a thrown exception the 
caller has no way to distinguish it from success.
   
   Two things are worth keeping separate in the fix, because they're easy to 
conflate:
   
   **Interrupt disposition and task disposition are orthogonal.** Restoring the 
interrupt flag (`Thread.currentThread().interrupt()`) is correct 
cooperative-thread hygiene — it tells the *current thread* it was asked to 
stop. But interrupt status is a signal about the *thread*, not a disposition 
for the *task*. The task still needs one of the three terminal states above; 
restoring the flag satisfies a different concern and cannot substitute for it. 
The asymmetry with `submit(Callable)` (which correctly returns a failed future) 
is really a symptom that the two overloads were never pinned to a shared 
terminal-state contract — each branch resolved the interrupted case locally and 
they diverged. The durable fix is to state that contract once (*any overload 
that accepts work must either execute it or surface a terminal failure — no 
fourth state exists*) and have both overloads derive from it, rather than 
patching the `Runnable` branch to mirror the `Callable` one by inspection (whic
 h would leave the same trap latent for the next overload someone adds).
   
   **The blast radius is what pushes this above "medium".** Resolver runs 
during dependency resolution. A silently-dropped resolution task means a build 
can report success against a stale or partially-resolved artifact graph — the 
build's *correctness* output is silently corrupted while every ordinary signal 
(exit code, logs unless you already know where to look) says "fine." For a tool 
whose whole value proposition is reproducible, trustworthy resolution, that 
inverts the guarantee. It's the same shape as a readiness/health probe that 
returns healthy regardless of outcome: the gate exists specifically to catch 
this class of problem, so when the gate itself lies there is no remaining 
signal downstream.
   
   A cheap guardrail that survives refactors is a round-trip invariant stated 
as a property: *for every path through `submit(Runnable)`, either the 
Runnable's effects are observable or an exception/future-completion is 
observable — no path exists in which neither holds.* If that property turns out 
to be hard to assert for the current API, that's precisely the signal that 
submission has grown a fourth state it was never supposed to have.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to