Is it okay to call TryAcquireSRWLock from a thread that has already acquired the lock?


A customer found the MSDN documentation ambiguous. Wouldn't be the first time.

In the description of Slim Reader/Writer Locks:

An SRW lock is the size of a pointer. The advantage is that it is fast to update the lock state. The disadvantage is that very little state information can be stored, so SRW locks cannot be acquired recursively. In addition, a thread that owns an SRW lock in shared mode cannot upgrade its ownership of the lock to exclusive mode.

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"?

The question was directed more specifically at the Try variants. If you "try" to acquire an SRW lock that the thread has already acquired, does the system detect this and cause the "try" to fail? Or is it a programming error?

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.

All the rules that apply to Acquire­SRW­Lock­* also apply to Try­Acquire­SRW­Lock­*. The only difference between the two is that if the lock cannot be acquired, the regular Acquire­SRW­Lock­* functions will block until the lock is acquired, whereas the Try­Acquire­SRW­Lock­* functions will return immediately and say, "Sorry. I would have blocked, but you asked me not to block."

I'll see what I can do to make this clearer in the documentation.

Comments (32)
  1. henke37 says:

    The article summary is hilarious in how blunt it can be at times. "No".

  2. Pietro Gagliardi (andlabs) says:

    I've read through the MSDN pages on SRW locks, but there's still one thing I don't know, and I'm not sure if I'm missing something elsewhere in the documentation or misread something: will the non-Try functions block until all holds of the other type release? For instance, if threads A and B call Acquire­SRW­Lock­Shared and thread C calls Acquire­SRW­Lock­Exclusive, will thread C be blocked until both thread A and B release their locks? And vice versa? My familiarity with RW locks in other languages tells me that this should be the case...

    1. "When the lock has been acquired in exclusive mode, no other thread can access the shared resource until the writer releases the lock." When thread C successfully gains exclusive access, threads A and B do not have access (which implies that they have left the lock). Though I can sort of see how this could be construed to say "Once you gain exclusive access, no *new* threads can access the resource (but existing threads with access still have access)." But if that were the case, then what would be the point of "exclusive" access?

      1. Pietro Gagliardi (andlabs) says:

        Yeah, so I was misreading it, with a bit of not thinking as well =P Thanks.

  3. rich says:

    MSDN has always been completely useless. Wouldn't you at least have someone doing it capable of writing clearly and unambiguously? I have high hopes that the new Stack Overflow Documentation project will render it redundant, and MS can pull the plug on the whole thing, rather than at present, where they just break all the links every few months.

    1. xcomcmdr says:

      > MSDN has always been completely useless.

      Not true, I find it way more descriptive and helpful than StackOverflow, 'cause on MSDN people actually know what they are talking about.

      A typical SO Question is : "how do I do this ?". That's fine, but the typical answer is : "Try this weird unrelated trick in order to do it ! It works on my end, dunno why lol".

      At times, MSDN is hard to read, but that's because documenting everything clearly for everyone is hard, and always has been. Or perhaps the reader does not have the necessary informations / prior readings in his/her head.

    2. Pietro Gagliardi (andlabs) says:

      I'd rather it not. SO Docs has a very different design and scope to MSDN, and its design does not satisfy every use case that's needed. SO Docs is example-driven, with little in the way of introductory or expository material, and vaguely defined syntax or notes sections. Examples are great, and formal documentation is often sorely lacking in them, but they're far from sufficient. The SO Docs design is great for some classes of documentation, but the people who want it to obsolete **all documentation ever** scare me.

    3. Brian says:

      Wow, you have high standards for something to escape being "useless". I've been using MSDN since the entire library was delivered to subscribers once a quarter on a single CD (when they went to 2 CDs, there were instructions on how to make the best of it, since CD players were expensive in those days.
      Yeah, MSDN documentation is written by humans, not all of whom have deep knowledge of every edge case of every call. But, is it "useless" - I can't imagine how you could call it that. Even when something happens and the links get busticated, it's usually not that hard to find what you are looking for using Google, Bing or some other search engine.
      And, it's sooooo much better than what passed for docs before the .NET Framework shipped.

    4. ta.speot.is says:

      I have found the MSDN documentation to be quite comprehensive most of the time. In fact, one bug I was responsible for causing would have been avoided if I'd read "Remarks" for ShellExecute more closely. (ShellExecute will initialise COM if you haven't already. So if you need a specific COM threading model you have initialise it before calling ShellExecute.)

      Some documentation (like Remote Desktop Services Dynamic Virtual Channels) is extremely terse to the point of being almost unworkable.

      Almost.

  4. Yukkuri says:

    I wouldn't assume a lock is recursive unless the documentation clearly said so... it seems like assuming a commuter car is amphibious and then complaining about the manual being unclear when you end up stuck in the lake.

  5. Wait, am I seriously not allowed to call these recursively? Rust legitimately relies on being able to safely call any of the acquire methods at any time even if it is already acquired in the same thread. If it seriously results in memory unsafe undefined behavior then that is a huge deal.

    According to a quick test, acquiring a shared lock when the thread already has a shared lock seems to simply work without issue. Meanwhile trying to acquire an exclusive lock when the thread has a shared lock or trying to acquire any lock when the thread has an exclusive lock will either deadlock or return WouldBlock depending on whether you used the Try versions or not.

    So what you're saying is that I cannot rely on this behavior at all and it is undefined behavior?

    1. 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.

      1. Rust currently relies on them being memory safe. So if by undefined behavior you mean that memory unsafety can occur, this means Rust cannot rely on SRWLocks anymore and will have to change its implementation to something else, such as a custom implementation on top of NT Keyed Events. Are keyed events de facto stable at this point? Can Rust rely on their API continuing to exist in the future and not changing?

        1. 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. Keyed events are not documented and therefore come with no stability guarantee.

          1. Allowing simultaneous exclusive acquisitions and causing stack memory corruption are definitely unsafe by Rust's definition, so Rust cannot use SRWLocks. And because keyed events have no stability guarantee this means that Rust cannot implement its own locks on top of them either. So basically Rust is screwed in this regard. Thanks Microsoft.

          2. Let me get this straight. You intentionally invoked undefined behavior, and now you're upset that the undefined behavior isn't undefined in the way that you like, and somehow that's Microsoft's fault.

          3. Nah, it is Rust's fault for not reading the documentation fully enough and understanding all the consequences.

            However, Rust does need to provide a safe RwLock that has safe behavior when attempting to lock it recursively, either deadlocking or returning an error, so it is Microsoft's fault that we cannot use either keyed events or SRW Locks for it. So now we're investigating what other options we have to implement RwLock safely.

          4. Okay, so it's Microsoft's fault that it wrote a synchronization primitive that doesn't exactly meets your needs. Gotcha.

          5. Peter, why can't you keep using SRW locks and just keep a separate (possibly thread-local) variable to tell you whether you've already acquired the lock or not?

          6. Billy O'Neal says:

            @Peter: You can use SRWLock to do it, you just need something like struct { SRWLOCK lock; uint32_t owningThreadId; uint32_t recursionCount; };. Lock then becomes: if (owningThreadId == currentThreadId()) { ++recursionCount } else { AcquireSRWLockExclusive(lock); owningThreadId = currentThreadId(); recursionCount = 1; }. (The implementation I wrote for the next major version of the Visual C++ synchronization primitives (not necessarily the next version of Visual Studio) does this for std::recursive_mutex on Vista+)

      2. Also to clarify, Rust considers deadlocking to be safe. If undefined behavior merely means it might deadlock, then Rust is okay with that.

        1. Pietro Gagliardi (andlabs) says:

          Are you sure SRWs and keyed events are your only options here? Windows has more synchronization objects than that...

          1. Ivan K says:

            >Windows has more synchronization objects than that…

            Exactly. Don't blame Microsoft because someone apparently hasn't heard of parts of the Windows API that have been around since the first version of Windows NT. 'Fair' or 'unfair' RW locks can be implemented as desired with other synchronisation objects.

        2. Zan Lynx' says:

          Ivan is right that you can implement your own RWLock. I've done it. It was _extremely_ slow.

          Microsoft's Mutex thing seems to be about 60x slower than a Linux futex. Could have done it faster with CriticalSections but I needed it to be inter-process. Tried the File Locking operations too, but they are no faster than Mutexes. Just sucked all around.

    2. Yukkuri says:

      Wow, your car is sunk in a lake. Totally not your fault assuming all cars are boats...

  6. Maybe use "shall not" instead of "cannot", with a link to https://www.ietf.org/rfc/rfc2119.txt?

  7. Jonathan Gilbert says:

    Hmm. Despite what the documentation says, is it in fact *possible* to design a SRW lock that a) uses exactly 1 pointer's storage, no more, and b) behaves differently from a given starting state depending on which thread is making a request? As far as I can see, the documentation of the behaviour being undefined when e.g. recursively acquiring a read lock must be more butt-covering than warning of any specific misbehaviour. One might say that it's keeping the door open for future changes in implementation, but unless those future changes expand the size of the lock's state past the size of a single pointer, I don't see a way for the behaviour to differ when a lock that is already acquired for read is acquired for read a second time, depending on whether it is the same thread or a different thread. There can be no concurrency concerns; a thread cannot be concurrent with itself. The lock is *already* safe with regard to concurrency between multiple threads, which is where the behaviour *is* defined. The more significant warning is that *because* the behaviour cannot differ depending on which thread is making the call, it isn't possible for a thread to upgrade or downgrade its lock in-place -- it must behave exactly as though the lock that it was trying to upgrade or downgrade was placed by any other thread in the process, which implies a deadlock.

    I wonder whether Microsoft might be able to task someone to review the code more closely and determine conclusively whether it is safe to recursively acquire read locks -- my *suspicion* is that it would require only a documentation change for that behaviour to be defined. :-)

    (This is not an endorsement of assuming a specific variety of undefined behaviour under the current state of the documentation, mind you.)

    1. But said documentation change would encourage improper use of the object, wouldn't it?

      1. Jonathan Gilbert says:

        I don't think so. If my theory is right, then the fact that it is improper to recursively acquire reader locks *is only part of the documentation*. If the underlying object is, in fact, perfectly capable of handling it, and a software engineer analyzes it closely and certifies that that is the case, then a documentation change that says that it is okay after all wouldn't be breaking any rules. The allowable behaviour under the new documentation would be a strict superset of the old documentation, so any code that was following the old rules would also be following the new rules. Allowing recursive lock acquisition makes it much simpler to tie use of the lock to scopes in code, e.g. with a class that acquires the lock in the ctor and releases it in the dtor. Is there some philosophical reason why one should inherently not recursively acquire locks, or is it simply an implementation detail (or documentation detail) of this particular implementation?

        1. Disallowing recursive acquisition is not an implementation detail, nor is it a documentation detail. it is a design detail. The lock was not designed to allow recursive acquisition, the code does nothing to explicitly support it, and recursive acquisition goes against the intended use of the lock. You seem to be arguing that the documentation should reflect the implementation, rather than the contract. I don't see why it is incumbent upon the manufacturer of a screwdriver to document which types of nails it can be used to pound safely.

          1. Jonathan Gilbert says:

            Sure, but the design is effectively another aspect of the documentation. The question is, is there anything gained by not speccing out recursive locks? If it turns out that the most straightforward implementation *does* handle them implicitly, then it isn't out of the question to go back to the design and augment it. Of course, the majority of the time, adding a new requirement to the design means that significant new engineering work will need to be done, and new code needs to be written. In this case, though, *if my theory is right*, the current implementation already fully implements -- correctly -- the feature requirement for recursive locking. So, then the next question is, even though we might not need to do new engineering work or write new code, updating the design isn't completely free; documentation would need to be changed, new automated tests written and verified and so on -- is it worth this time and effort to support recursive locking? In my opinion, it *is* worth it. Explicitly supporting recursive locking allows for stronger, more reliable and more maintainable software development patterns to be used in code that uses this feature. It makes it a better feature, and it makes the code that uses the feature less likely to "do it wrong". Fewer bugs is better for everyone. So, if the current implementation *already* provides the functionality, in my opinion the benefits outweigh the time and effort needed to update the contract and the documentation. I don't expect my opinion to shift Microsoft's priorities at this point, but I don't think I'm wrong either. :-)

          2. Determining whether the current implementation supports recursive acquisition would probably take a long time, seeing as it wasn't designed in, and you would have to go back and redo all the correctness analysis. The code has likely not been modified in years, so the expertise is no longer in developer short-term memory; it's in documents that need to be re-read and re-understood. And the original designers would likely argue that if your code does recursive acquisition, then it's already doing it wrong, and we shouldn't go around explicitly telling it's people it's okay to do something wrong.

Comments are closed.

Skip to main content