Let's rely on undefined behaviour for memory safety



  • I'm sure some of you saw this on The Old New Thing as well, but I couldn't find anything on it here, so.

    👮 Raymond Chen
    😕 Peter Atashian

    👮 (in article): 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.

    😕: 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.

    👮: The fact that they cannot be acquired recursively is what makes them slim! Attempting to acquire recursively results in undefined behavior.

    😕: 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?

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

    😕: [...] 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.

    👮: 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.

    😕: Nah, it is Rust’s fault for not reading the documentation fully enough and understanding all the consequences. However, [...] it is Microsoft’s fault that we cannot use either keyed events or SRW Locks for it.

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

    👨: Are you sure SRWs and keyed events are your only options here? 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.

    I particularly like the way the first reaction to "OMG we're invoking undefined behaviour here" is "let's use an undocumented feature instead!" Yeah, there's no way that could go wrong.


  • Impossible Mission Players - A

    Whoa, it's like deja vu but with higher definition!


  • BINNED

    @Scarlet_Manuka said in Let's rely on undefined behaviour for memory safety:

    let's use an undocumented feature instead!"

    Undocumented is different from poorly documented. MSDN is usually great (the best to be honest), but not in this case.

    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.

    Cannot? or must not? cannot when reading about Trying to lock could very easily be interpreted as will fail



  • @dse That's actually what the article was about in the first place: clarifying that cannot here should be interpreted as must not.

    The undocumented feature part refers to the keyed events.



  • @dse said in Let's rely on undefined behaviour for memory safety:

    Undocumented is different from poorly documented.

    Another quote from that link @bb36e posted in the Funny Stuff:

    Documentation is like sex: when it is good, it is very, very good; and when it is bad, it is better than nothing. ― Dick Brandon



  • @Scarlet_Manuka said in Let's rely on undefined behaviour for memory safety:

    Wait, am I seriously not allowed to call these recursively?

    On a side-note, Linux kernel internally uses semaphores as mutexes. It should be rather obvious that these are not recursive either (though in this case the behaviour is defined: the thread will deadlock). And Linus militantly refuses to include any kind of mutexes. Because synchronization rules under which it might not be clear whether you hold the lock or not are likely to be wrong anyway.

    And he's right. In fact, the Rust safety rules require that it's Mutex is not recursive. If it was, it would allow a child function to get a mutable reference to the protected object even if its caller already has one, but there may only be one outstanding mutable reference to any object at any time.

    Having undefined behaviour is a bit of a problem though. Rust needs such attempt to reliably fail.



  • @Bulb - I'm wondering - can this be solved by just storing a thread id (containing the current locking thread's id) next to the SRW and checking it before locking & setting it after locking & clearing it before unlocking?
    Given some very basic concurrency guarantees (thread id is atomic, writes in a thread are visible in the same thread), this should work, no?



  • @CreatedToDislikeThis said in Let's rely on undefined behaviour for memory safety:

    I'm wondering - can this be solved by just storing a thread id (containing the current locking thread's id) next to the SRW and checking it before locking & setting it after locking & clearing it before unlocking?
    Given some very basic concurrency guarantees (thread id is atomic, writes in a thread are visible in the same thread), this should work, no?

    The SRWs have a shared mode, so multiple threads can hold the lock (i.e., for read access). Makes storing the thread IDs a fair bit more difficult.



  • @cvi said in Let's rely on undefined behaviour for memory safety:

    @CreatedToDislikeThis said in Let's rely on undefined behaviour for memory safety:

    I'm wondering - can this be solved by just storing a thread id (containing the current locking thread's id) next to the SRW and checking it before locking & setting it after locking & clearing it before unlocking?
    Given some very basic concurrency guarantees (thread id is atomic, writes in a thread are visible in the same thread), this should work, no?

    The SRWs have a shared mode, so multiple threads can hold the lock (i.e., for reading). Makes storing the thread IDs a fair bit more difficult.

    Oh - doh, missed that they were SRWs!
    That's indeed trickier and in fact pthread's rwlocks don't guarantee too much about them either. (pthread only defines what happens when a thread acquires the shared lock while holding the shared lock - all other such cases are undefined, and the case of acquiring the exclusive lock while holding the shared lock can't be solved with a single thread id either).



  • That said I would wish that more lock implementations had debug versions of their lock that'd check for any wrong usage of the API and abort on it, even if this is more expensive.
    It's nice to say "it's undefined behaviour", but when you have no earthly idea whether and where you're triggering it, that's not very helpful.



  • @CreatedToDislikeThis said in Let's rely on undefined behaviour for memory safety:

    That said I would wish that more lock implementations had debug versions of their lock that'd check for any wrong usage of the API and abort on it, even if this is more expensive.
    It's nice to say "it's undefined behaviour", but when you have no earthly idea whether and where you're triggering it, that's not very helpful.

    Are you complaining that if you don't know what you are doing, things that you don't know what they are will happen? Are you surprised by this?



  • @CreatedToDislikeThis said in Let's rely on undefined behaviour for memory safety:

    It's nice to say "it's undefined behaviour", but when you have no earthly idea whether and where you're triggering it, that's not very helpful.

    To be honest I am somewhat surprised these things are UndefinedBehaviour™. Semaphore is as simple as it gets, can be used for implementing mutexes, both normal and R/W (initial value is maximum, lock_shared is down(1) and lock_exclusive is down(max)) and attempt to lock such thing recursively is well defined—it deadlocks. Can anybody see how allowing UB™ can make the lock any simpler?



  • @Steve_The_Cynic said in Let's rely on undefined behaviour for memory safety:

    @CreatedToDislikeThis said in Let's rely on undefined behaviour for memory safety:

    That said I would wish that more lock implementations had debug versions of their lock that'd check for any wrong usage of the API and abort on it, even if this is more expensive.
    It's nice to say "it's undefined behaviour", but when you have no earthly idea whether and where you're triggering it, that's not very helpful.

    Are you complaining that if you don't know what you are doing, things that you don't know what they are will happen? Are you surprised by this?

    No, I'm complaining that if I do know what I'm doing but am not omniscient, things that I don't know what will happen and I may well not know about them, depending on what they are.
    Are you claiming omniscience? Zero-bugs code?



  • @CreatedToDislikeThis Agreed, having a debug-version that you can conditionally compile in would be nice.



  • @CreatedToDislikeThis, @cvi, well, the SRW locks are lighter version of other synchronization primitives that there also are. So you can develop with the heavier-weight ones that do have debug mode and then #define over to the SRW ones for optimized build...



  • @Bulb The point about having it in by default in e.g., debug builds (much like the iterator checking that VS does/used to do), is that the misuse by rust would have been detected more or less immediately. But I agree with your point that the SRW locks probably qualify as a sufficiently low-level primitive that weighing them down even in debug mode may be a bad choice.

    Out of curiosity, I checked std::mutex::lock, which states

    If lock is called by a thread that already owns the mutex, the behavior is undefined: the program may deadlock, or, if the implementation can detect the deadlock, a resource_deadlock_would_occur error condition may be thrown.

    Does that mean that the implementation must either deadlocks or throws, or is the implementation allowed to do some random other things too?



  • @CreatedToDislikeThis said in Let's rely on undefined behaviour for memory safety:

    @Steve_The_Cynic said in Let's rely on undefined behaviour for memory safety:

    @CreatedToDislikeThis said in Let's rely on undefined behaviour for memory safety:

    That said I would wish that more lock implementations had debug versions of their lock that'd check for any wrong usage of the API and abort on it, even if this is more expensive.
    It's nice to say "it's undefined behaviour", but when you have no earthly idea whether and where you're triggering it, that's not very helpful.

    Are you complaining that if you don't know what you are doing, things that you don't know what they are will happen? Are you surprised by this?

    No, I'm complaining that if I do know what I'm doing but am not omniscient, things that I don't know what will happen and I may well not know about them, depending on what they are.
    Are you claiming omniscience? Zero-bugs code?

    Good grief, not at all. Sounds like you're saying you don't know what the code's doing, though. Or that it was badly designed, and it is having problems because of this.



  • @cvi said in Let's rely on undefined behaviour for memory safety:

    @Bulb The point about having it in by default in e.g., debug builds (much like the iterator checking that VS does/used to do), is that the misuse by rust would have been detected more or less immediately. But I agree with your point that the SRW locks probably qualify as a sufficiently low-level primitive that weighing them down even in debug mode may be a bad choice.

    Out of curiosity, I checked std::mutex::lock, which states

    If lock is called by a thread that already owns the mutex, the behavior is undefined: the program may deadlock, or, if the implementation can detect the deadlock, a resource_deadlock_would_occur error condition may be thrown.

    Does that mean that the implementation must either deadlocks or throws, or is the implementation allowed to do some random other things too?

    The behaviour is undefined. It is allowed to, for example, format the system drive, and its behaviour, while undesirable, would be within the specification. Look up "nasal demons" on Google, but the summary is that as soon as you stray into territory marked out by the sign "Here Be Undefined Behaviour" (aka UB), all bets are off. Even the mechanism of betting is off.


  • Impossible Mission - B

    @Steve_The_Cynic said in Let's rely on undefined behaviour for memory safety:

    Tthe summary is that as soon as you stray into territory marked out by the sign "Here Be Undefined Behaviour" (aka UB), all bets are off. Even the mechanism of betting is off.

    As Raymond Chen has put it in the past, (loosely paraphrasing here,) any result is valid as undefined behavior, including your code working as intended. That still doesn't mean it's right.



  • @cvi said in Let's rely on undefined behaviour for memory safety:

    Does that mean that the implementation must either deadlocks or throws, or is the implementation allowed to do some random other things too?

    That means whoever wrote the docs isn't familiar with common terminology.

    Unspecified behavior is when you have a limited set of options and while the implementation can choose either, or all of them, without documenting the choice, it won't do anything else.

    Undefined means nasal demons, and makes the latter part pointless since there's no guarantee the implementation won't do anything that's not on the list.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.