0

Platform Debian 9 | ARM

I have a class that is used to store thread IDs on a std::list and then print the list as part of the shutdown procedure. There are two static methods: AddId() and PrintIds(). Since the static methods are used by several threads, the list access is protected with a std::mutex. I have come across two issues that have me baffled and they may be related. I hope one of you can help explain what I am missing.

Note I first used scoped std::lock_guard<std::mutex>s and have temporarily settled on the std::unique_lock<std::mutex>s that are shown in the code for the reasons explained herein.

Issue 1 When the program starts, all threads call ThreadMgr::AddId(), which adds the thread id to the list along with a string name. That seems to work fine. However, when ThreadMgr::PrintIds() is called during the shutdown procedure, the std::mutex is still locked and the std::lock_guard blocks. While testing, I unlocked the std::mutex in the line preceding the std::lock_guard--which is undefined behavior if it wasn't locked by the calling thread--and then it worked as expected. The only place the std::mutex is used is in these two methods, and with the std::lock_guard being scoped, the std::mutex should have been unlocked with every method call. I went through several iterations of locking and unlocking the std::mutex directly, using std::unique_locks, etc. I eventually found a somewhat workable solution using the std::unique_locks that are in the code provided here. However, that too has me baffled.

The std::unique_locks work if I use std::adopt_lock, but only in ThreadMgr::PrintIds(). If I use std::adopt_lock in ThreadMgr::AddId(), then I get a segmentation fault whenever ThreadMgr::PrintIds() is called.

Issue 2 As previously stated, ThreadMgr::AddId() runs fine with either std::lock_guard or std::unique_lock when all threads call it on startup. However, there are UI sessions where each session spawns a thread. When ThreadMgr::AddId() is called from one of these session threads, both std::lock_guard and std::unique_lock block like the std::mutex is already locked. So, ThreadMgr::AddId() works perfectly fine for every other thread, but not for the session threads that are started later.

If there is any other information I can provide to help get to a solution, let me know.


class ThreadMgr {
 public:
    ThreadMgr();
    ~ThreadMgr();
    static void AddId(std::string);
    static void PrintIds();

 private:
    static std::list<ThreadId> t_list;
    static std::mutex list_mtx;
};

/*Definition*/
ThreadMgr::ThreadMgr() {}
ThreadMgr::~ThreadMgr() {}

/*Static Members*/
void ThreadMgr::AddId(std::string name) {
    
    ThreadId t_id = getThreadId(name);
    {
        std::unique_lock<std::mutex> lock(list_mtx);
        t_list.push_front(t_id);
    }

    std::lock_guard<std::mutex> lk(cout_mtx);
    std::cout << "---Added id: " << t_id.tid << " for the " << name << " thread." << std::endl;

    return;
}
void ThreadMgr::PrintIds() {
    
    std::ostringstream oss;
    oss << "\n**************************Thread Ids**************************\n";
    
    {   
        std::unique_lock<std::mutex> lock(list_mtx, std::adopt_lock);
        
        for (std::list<ThreadId>::iterator t = t_list.begin(); t != t_list.end(); t++) {
            oss << "---" << t->name << " : " << t->tid << '\n';
        }
    }
    oss << "************************End Thread Ids************************" << '\n';
            
    std::lock_guard<std::mutex> lk(cout_mtx);
    std::cout << oss.str() << std::endl;
    
    return;
}

std::mutex ThreadMgr::list_mtx;
std::list<ThreadId> ThreadMgr::t_list;

int Main(){

    ThreadMgr::AddId("Main");

    std::thread MbServerT(MbServer);
    std::thread UiServerT(UiServer);

    while(run){
        //do stuff
    }
    
    ThreadMgr::PrintIds();
    
    if(MbServerT.joinable())
        MbServerT.join();

    if(UiServerT.joinable())
        UiServerT.join();

    return 0;
}
Jerry
  • 41
  • 5
  • 3
    Can you add a `main` function and such, so that it's a [mcve] that people can test? – Nate Eldredge Jan 14 '22 at 05:24
  • You might also like to throw ThreadSanitizer at it and see if it provides any hints. – Nate Eldredge Jan 14 '22 at 05:25
  • In `ThreadMgr::PrintIds()`, why do you use `std::adopt_lock` argument in `std::unique_lock` ctor? – Eugene Jan 14 '22 at 05:42
  • Are you doing any of these operations before `main` is entered or after `main` returned? – user17732522 Jan 14 '22 at 05:48
  • @Nate, I've added a basic `main`, but it's not minimal reproducible by copying/pasting. I will work on that in a bit. Thanks for the ThreadSanitizer suggestion. I am working through that now. – Jerry Jan 15 '22 at 02:59
  • @Eugene, I landed at the `std::adopt_lock` as I was looking for a solution; it did not make sense to me. With Nate's ThreadSanitizer suggestion, I now know that it is actually unlocking the lock in an undefined manner. So, while it gets the immediate result I need, it's undefined behavior. – Jerry Jan 15 '22 at 03:01
  • @ user17732522, no. The threads that are spawn are done so in main() and the methods are called before main() exits. In addition, all threads are joined before exiting main(). – Jerry Jan 15 '22 at 03:07

0 Answers0