Xilinx / XRT

Run Time for AIE and FPGA based platforms
https://xilinx.github.io/XRT
Other
558 stars 473 forks source link

lock_guard vs unique_lock in destructor #5436

Closed KarimH22 closed 3 years ago

KarimH22 commented 3 years ago

Hello I met an issue on xrt stop, the scheduler is blocked inside kds stop looking cplusplus reference about condition_variable https://cplusplus.com/reference/condition_variable/condition_variable/?kw=condition_variable in destructor it is used unique_lock and not lock_guard

`#0 0x00007f0ec7b0498d in pthread_join () from /lib/x86_64-linux-gnu/libpthread.so.0

1 0x00007f0ec8ccab97 in std::thread::join() ()

from /usr/lib/x86_64-linux-gnu/libstdc++.so.6

2 0x00007f0ec84187c9 in xrt::kds::stop() ()

from /opt/xilinx/xrt/lib/../lib/libxrt++.so.2

3 0x00007f0ec841d195 in xrt::scheduler::stop() ()

from /opt/xilinx/xrt/lib/../lib/libxrt++.so.2`

so what I changed is


diff --git a/src/runtime_src/xrt/scheduler/kds.cpp b/src/runtime_src/xrt/scheduler/kds.cpp index 907173c..1665272 100644 --- a/src/runtime_src/xrt/scheduler/kds.cpp +++ b/src/runtime_src/xrt/scheduler/kds.cpp \@@ -229,7 +229,7 @@ stop() return; { - std::lock_guard lk(s_mutex); + std::unique_lock lk(s_mutex); s_stop = true;
}


and then no blocking point

stsoe commented 3 years ago

Hello @KarimH22 Thanks for your report.

std::unique_lock is only required by the std::condition_variable::wait functions. It is not needed by code that intends to modify a shared variable. See https://en.cppreference.com/w/cpp/thread/condition_variable for more details.

As such you are running into some other problem that may be a bug in XRT or some place else. At any rate, the code you modified has been removed / reorganized from XRT.

KarimH22 commented 3 years ago

Hi @stsoe indeed no issue seen with last version 2021.1 2.11.634 thank you