Remove OS Thread from Atomics Wait Async#989
Conversation
aapoalas
left a comment
There was a problem hiding this comment.
Generally looks good to me, but I had a few nitpicks regarding the inline comments that I'd like to fix before merging :)
|
|
||
| // SAFETY: buffer is a cloned SharedDataBlock; non-dangling. | ||
| let waiters = unsafe { self.0.data_block.get_or_init_waiters() }; | ||
| // a. Perform EnterCriticalSection(WL). |
There was a problem hiding this comment.
nitpick (but blocking): I'd like to see here a reference to where these steps are coming from. Possibly also a link to the relevant spec method.
| )); | ||
| } | ||
| } | ||
| // c. Perform LeaveCriticalSection(WL). |
There was a problem hiding this comment.
nitpick (blocking): Step b is missing. I assume that'd be removing the waiter from the list, but I'd want to see it.
| gc, | ||
| )); | ||
| } | ||
| WaitResult::TimedOut => { |
There was a problem hiding this comment.
thought: The string could be created from some WaitResult::to_string method to combine the two paths into a single try_resolve(agent, result.to_string(), gc).
| } | ||
|
|
||
| // SAFETY: buffer is a cloned SharedDataBlock; non-dangling. | ||
| let waiters = unsafe { self.0.data_block.get_or_init_waiters() }; |
There was a problem hiding this comment.
nitpick: This seems like it could be a helper method.
No description provided.