google / sanitizers

AddressSanitizer, ThreadSanitizer, MemorySanitizer
Other
11.5k stars 1.04k forks source link

TSAN False Positive With std::unique_lock::try_lock_for and std::unique_lock::try_lock_until #1620

Open kyle-figure opened 1 year ago

kyle-figure commented 1 year ago

Description: A false positive occurs when using either try_lock_for or try_lock_until with a std::timed_mutex object. In the code snippet provided, the mutex can only be unlocked by the thread which locks it, yet ThreadSanitizer reports a warning. I have tried using std::timed_mutex with the simple try_lock function, and it works without ThreadSanitizer reporting a warning.

Platform: Ubuntu 22.04 AMD64

main.cpp

#include <mutex>
#include <chrono>
int main()
{
    std::timed_mutex m_;
    std::unique_lock<std::timed_mutex> lock_(m_, std::defer_lock);
    if (lock_.try_lock_for(std::chrono::milliseconds(500)))
    {
        lock_.unlock();
    }
}

Compile command:

clang++ main.cpp  --std=c++20 -fsanitize=thread -O0 -g -fno-omit-frame-pointer

Execution output:

$ ./a.out
LLVMSymbolizer: error reading file: No such file or directory
==================
WARNING: ThreadSanitizer: unlock of an unlocked mutex (or by a wrong thread) (pid=1)
    #0 pthread_mutex_unlock <null> (a.out+0x6d13a) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #1 __gthread_mutex_unlock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:779:12 (a.out+0xd04b3) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #2 std::timed_mutex::unlock() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/mutex:255:7 (a.out+0xd1545) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #3 std::unique_lock<std::timed_mutex>::unlock() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/unique_lock.h:195:17 (a.out+0xd0786) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #4 main /home/main.cpp:9:15 (a.out+0xd0418) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)

  Location is stack of main thread.

  Location is global '??' at 0x7ffe683ee000 ([stack]+0x1f180)

  Mutex M0 (0x7ffe6840d180) created at:
    #0 pthread_mutex_unlock <null> (a.out+0x6d13a) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #1 __gthread_mutex_unlock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:779:12 (a.out+0xd04b3) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #2 std::timed_mutex::unlock() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/mutex:255:7 (a.out+0xd1545) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #3 std::unique_lock<std::timed_mutex>::unlock() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/unique_lock.h:195:17 (a.out+0xd0786) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)
    #4 main /home/main.cpp:9:15 (a.out+0xd0418) (BuildId: 154e31637c56a373a26a01e3419719138e465df8)

SUMMARY: ThreadSanitizer: unlock of an unlocked mutex (or by a wrong thread) (/home/a.out+0x6d13a) (BuildId: 154e31637c56a373a26a01e3419719138e465df8) in __interceptor_pthread_mutex_unlock
==================
ThreadSanitizer: reported 1 warnings

clang++ version:

$ clang++ --version
Ubuntu clang version 14.0.0-1ubuntu1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
soblin commented 7 months ago

Isn't it because the destructor of lock variable also calls m.unlock() at the end of the scope which means the lock_ is unlocked twice?

kyle-figure commented 7 months ago

Isn't it because the destructor of lock variable also calls m.unlock() at the end of the scope which means the lock_ is unlocked twice?

The unique lock object does not hold the mutex after instantiation, due to std::defer_lock. If what you are saying was the issue here, try_lock should also fail in the same way. However, try_lock works and try_lock_for does not.

soblin commented 7 months ago

However, try_lock works and try_lock_for does not.

Okay, that's weird. BTW in my understanding, unique_lock with defer_lock just skips initial lock() at instantiation and ensures unlock() at destruction in RAII manner.

serpent7776 commented 6 months ago

Is there update on this? I just hit the same issue.

eiselekd commented 2 months ago

Same here with -fsanitize=thread will throw a tsan error at the first iteration:

#include <chrono>                                                                                                                            
#include <mutex>                                                                                                                             
#include <thread>                                                                                                                            

using namespace std::chrono_literals;                                                                                                        

int main(int argc, char **argv) {                                                                                                            
    (void)argc; (void)argv;                                                                                                                  
    std::timed_mutex m;                                                                                                                      

    while (true) {                                                                                                                           
        if (m.try_lock_for(100ms))                                                                                                           
        {                                                                                                                                    
            m.unlock();                                                                                                                      
        }                                                                                                                                    
        std::this_thread::sleep_for(5ms);                                                                                                    
    }                                                                                                                                        

    return 0;                                                                                                                                
}                                                                                                                                            

while the same with valgrind --tool=helgrind works fine. I use llvm version 20.