Tweak SemaphoreDisposer to null out the held semaphore #57831
Conversation
While investigating a deadlock, it would have been made the analysis easier if we null out the semaphore once we've released it, because it means the then one-and-only instance of a SemaphoreDisposer that points to the semaphore is the one that is holding the lock.
| // async deadlock where a SemaphoreSlim is held but you're unsure why, as long all the users of the | ||
| // SemaphoreSlim used the Disposable helpers, you can search memory and find the instance that | ||
| // is pointing to the SemaphoreSlim that hasn't nulled out this field yet; in that case you know | ||
| // that's holding the lock and can figure out who is holding that SemaphoreDisposer. |
There was a problem hiding this comment.
we're in a struct. so this does only work as long as the NonCopyable part above really is maintained. i can allow this :)
There was a problem hiding this comment.
@CyrusNajmabadi Indeed, the [NonCopyable] being violated does mean we could end up with "surprises", but at this point, this is the least of your concerns IMHO. 😄
There was a problem hiding this comment.
so this does only work as long as the NonCopyable part above really is maintained
Instances of this type are copied twice in this very file so [NonCopyable] is more like [NonCopyableForSomeVersionsOfCopy]
There was a problem hiding this comment.
Instances of this type are copied twice in this very file
There are no copies of this type in this file (moves are not copies). A copy is a case where the new location and the old location can still be accessed with the value; the analyzer ideally would ignore cases where the old location is accessible but never accessed, but it doesn't pay attention to that right now.
There was a problem hiding this comment.
There are two returns which are copies. You label them moves but that doesn't change that copies occur.
|
@jasonmalinowski compiler code needs 2 sign offs from the team |
|
@333fred Oops, thanks. I was staring at this going "oh, I should get a second compiler review" and didn't realize I had already set auto-merge. |
ghost
added this to the 
While investigating a deadlock, it would have been made the analysis easier if we null out the semaphore once we've released it, because it means the then one-and-only instance of a SemaphoreDisposer that points to the semaphore is the one that is holding the lock.