Bug #7617
closedcreation and destruction of @bindSafe@ functor is not thread safe
0%
Description
When creating and destroying multiple background threads using bindSafe
onto the same widget, the Wt::Core::Impl::observer_info::observers_
vector of that widget can get corrupted.
Wt: 4.3.1
boost: 1.73
MSVC: 16.6.1
Files
Updated by Roel Standaert over 4 years ago
It wasn't actually made to be thread safe in the first place. I guess the documentation gave you the idea that it would be thread safe, though?
Updated by Marco Kinski over 4 years ago
using it with WServer::post
is wrong or only the creation/destruction without holding the corresponding UpdateLock
?
Updated by Roel Standaert over 4 years ago
You can't use bindSafe
or copy the resulting function object without holding the corresponding UpdateLock
(you're copying finishcb
on every post
). I think you should be able to post something that uses bindSafe
unless there's an extra copy of the callback somewhere in Boost Asio that I'm not aware of. I think the destruction should be fine, since the observable will first clear its observers (while holding the update lock), turning the destruction of the observing_ptr
into a no-op. As I'm writing this I'm actually starting to realize that there's also the case where the observing_ptr
is destroyed before the observable, and I'd have to check to make sure if that use is safe.
Note: the reference capture is probably fine in this case, but I think it's best to capture this
instead of risking the capture of anything that will go out of scope.
Updated by Roel Standaert over 4 years ago
As it is currently, it appears that an observing_ptr
can indeed not be part of the function object that you pass to post()
as a callback.
WebController::handleApplicationEvent
calls WebSession::queueEvent
without the update lock, and queueEvent
causes a copy of the ApplicationEvent
, which contains the callback, including the observing_ptr
that bindSafe
uses.
Updated by Roel Standaert over 4 years ago
This actually stops being an issue when we use a std::shared_ptr
for the ApplicationEvent
. Maybe we should just do that, surely the overhead is acceptable. That doesn't change the fact that you do need to have the UpdateLock
at the moment you create your observing_ptr
(i.e. call bindSafe
), though!
Updated by Roel Standaert over 4 years ago
- Status changed from New to Implemented @Emweb
I changed ApplicationEvent
to use shared_ptr
.
Updated by Roel Standaert over 4 years ago
- Status changed from Implemented @Emweb to Resolved
Updated by Roel Standaert over 4 years ago
- Status changed from Resolved to Closed
Updated by Marco Kinski almost 4 years ago
While looking into #7881 I had a regression reproducing this bug again.