Disable early-out notify optimizations #139
Conversation
The notify() function contains two optimizations: - If the `inner` field is not yet initialized (i.e. no listeners have been created yet), it returns `0` from the function early. - If the atomic `notified` field indicates that the current notification would do nothing, it returns `0` early as well. `loom` testing has exposed race conditions in these optimizations that can cause notifications to be missed, leading to deadlocks. This commit removes these optimizations in the hope of preventing these deadlocks. Ideally we can restore them in the future. Closes #138 cc #130 and #133 Signed-off-by: John Nunley <dev@notgull.net>
|
r? @zeenix, as this is the conclusion of the deep dive I've been doing for the past couple weeks, as mentioned here: #130 (comment) r? @fogti, as per the C memory model I'm relatively sure these optimizations should have any effect on the order of events, but I'm not sure. |
|
I've tested I'm unclear why this bug is manifesting now; pretty much all versions of |
|
No, these optimizations were there to add fast paths that didn't lock the inner mutex and traverse the linked list. This operation is somewhat expensive so this optimization avoided doing that if it could. Removing it won't wake up any additional tasks. |
The notify() function contains two optimizations:
innerfield is not yet initialized (i.e. no listeners have beencreated yet), it returns
0from the function early.notifiedfield indicates that the current notification woulddo nothing, it returns
0early as well.loomtesting has exposed race conditions in these optimizations that can causenotifications to be missed, leading to deadlocks. This commit removes these
optimizations in the hope of preventing these deadlocks. Ideally we can restore
them in the future.
Closes #138
cc #130 and #133