Slim Reader Writer locks are not reentrant

[billti]

This should be clearer in the doc, as the talk about a "thread" owning and releasing a lock makes it sound like they are reentrant, when they are not.

[PRMerger9]

@billti : Thanks for your contribution! The author(s) have been notified to review your proposed change.

[PRMerger14]

@mcleanbyron : Thanks for your contribution! The author(s) have been notified to review your proposed change.

[jaykrell]

I'm not sure reentrant is the right term here.
People like to say "recursive", but I'm not sure that is right either.

[mcleanbyron]

Thanks for your feedback. I revised the text from "...are not re-entrant" to "...cannot be acquired recursively". This change should be published to the live docs later today.

[jaykrell]

I have not figured out how to report this, but separate issue in nearby docs: it is not clear if spurious wakes report success or failure, in the nearby condition variable documentation.

[jaykrell]

Also please run the “reentrant” vs. “recursive” vs. other by others. I not sure either is best. Maybe spell it out and give it a name, if not already- a thread holding a lock cannot again acquire it, without first releasing it. In contrast to EnterCriticalSection. Etc. maybe code samples..

[jaykrell]

I guess that already spelled out sorry

[RalfJung]

The changes performed in this PR are in direct contradiction to statements by Raymond Chen in this discussion (unfortunately only available on archive.org it seems). People there are debating what "cannot be acquired recursively" means. Quoting from there:

The ambiguity is over the word "cannot", used twice in the above paragraph. Does it mean "The system will not allow an SRW lock to be acquired recursively or to be upgraded from shared to exclusive"? Or does it mean "If you attempt to acquire an SRW lock recursively, or if you try to upgrade an SRW lock from shared to exclusive, then the result is undefined"?

[...]

It's a programming error. It is your responsibility as a programmer not to call Acquire­SRW­Lock­Shared or Acquire­SRW­Lock­Exclusive from a thread that has already acquired the lock. Failing to comply with this rule will result in undefined behavior.

Then later Peter Atashian asks explicitly

Wait, am I seriously not allowed to call these recursively? [...] So what you’re saying is that I cannot rely on this behavior at all and it is undefined behavior?

and the answer by Raymond Chen is

The fact that they cannot be acquired recursively is what makes them slim! Attempting to acquire recursively results in undefined behavior. Mind you, std::shared_mutex has the same undefined behavior on recursive acquisition.

and

Not sure what you mean by “memory safe”; are we talking about memory barriers? The behavior is undefined. It might deadlock. It might succeed. It might (and probably will) cause the lock to fail to fulfil its contract in the future (e.g,. allow two simultaneous exclusive acquisitions). And since wait nodes are threaded on the stack, it can result in stack memory corruption.

So has the situation changed since then, or did someone misinterpret the docs, or what is happening here?

This is quite important for us in Rust, we'd really like to be able to rely on the fact that trying to acquire an SRW lock in exclusive mode when the current thread already holds the lock (in either shared or exclusivce mode) will definitely deadlock and definitely not allow two simultaneous exclusive acquisitions (or, worse, cause arbitrary UB).

[mcleanbyron]

I'm sorry it took so long to reply. I reviewed the doc changes in this PR with an engineer who works in this part of the Windows API code base. The engineer confirmed that the changes correctly describe the current behavior, and also provided the following additional statements. I hope this helps.

---

"That happens to be the behavior today, but one can envision a world in which lock ownership is tracked via external means and illegal recursive acquisitions could be detected (e.g. when running under app verifier this could result in a controlled crash of the process).

An attempted exclusive recursive acquire via AcquireSRWLockExclusive will:

1. Not corrupt the lock (the lock will continue to uphold the locking contract)
2. Not result in a successful acquisition (the thread will block indefinitely or the process will be terminated in a controlled fashion)"

[RalfJung]

@mcleanbyron thanks! This matches the understanding of the docs on the Rust side.