fix(publish): avoid false deadlock when to_confirm is non-empty#17071
Conversation
rustbot
added
A-interacts-with-crates.io
|
r? @weihanglo rustbot has assigned @weihanglo. Use Why was this reviewer chosen?The reviewer was selected based on:
|
raushan728
force-pushed
the
fix/publish-workspace-deadlock-17028
branch
from
44e473d to
9072b53
Compare
This comment has been minimized.
This comment has been minimized.
raushan728
force-pushed
the
fix/publish-workspace-deadlock-17028
branch
from
9072b53 to
480f0e1
Compare
| .add_responder("/index/1/c", move |req, server| { | ||
| let mut lock = arc.lock().unwrap(); | ||
| *lock += 1; | ||
| if *lock <= 6 { |
There was a problem hiding this comment.
I had a hard time following this. Why is the magic number 6? Is there a way to document/model it better?
There was a problem hiding this comment.
This follows the same Arc<Mutex<>> pattern from wait_for_first_publish. The threshold is 6 instead of 1 because workspace publish queries c's index multiple times during the packaging phase before entering the wait loop. Should I add a comment explaining this if that helps?
There was a problem hiding this comment.
At least it should say where each query is from and for what purpose, like why querying 6 times. IIRC some other network request tests also document similar stuff
There was a problem hiding this comment.
This sometimes gets messy where you don't know why a query is added.
There was a problem hiding this comment.
Added comments explaining where each query comes from.
raushan728
force-pushed
the
fix/publish-workspace-deadlock-17028
branch
from
480f0e1 to
0721b4d
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
raushan728
force-pushed
the
fix/publish-workspace-deadlock-17028
branch
from
0721b4d to
ae3b65d
Compare
|
@weihanglo thoughts on beta backport? |
rustbot
removed
the
S-waiting-on-review
|
Yeah. Given the fix is minimal I think it is not too risky to have a beta-backport, though surprisingly that #17028 seems to have less reports than I expected. |
4 participants
Fixes #17028
PR #16722 introduced a check that fires an internal error when
readyis empty inside the publish loop. It fired even when packages were still legitimately waiting for registry confirmation into_confirm, causing valid workspace publishes to fail with a false deadlock error. A true deadlock only exists when bothreadyandto_confirmare empty.