WaitForSingleObject(mutex) must handle WAIT_ABANDONED

[huyuguang@…]

boost-1_54\boost\interprocess\sync\windows\winapi_mutex_wrapper.hpp line 53

void lock() {

if(winapi::wait_for_single_object(m_mtx_hnd, winapi::infinite_time) != winapi::wait_object_0){

error_info err = system_error_code(); throw interprocess_exception(err);

}

}

The wait_for_single_object maybe return wait_abandon which means the mutex holder thread exited and did not release the mutex. Normally, this should never happen, but if the mutext holder process crash, this will happen.

So the code should change to: unsigned long ret = winapi::wait_for(...); if(ret != winapi::wait_object_0 && ret != winapi::wait_abondon) {

error_info err = system_error_code(); throw interprocess_exception(err);

}

[huyuguang@…]

I think maybe I am wrong. Here the code should not handle the wait_abondon and should throw a exception. The caller can catch the exception and check if it should handle the exception or just ignore it.

[Ion Gaztañaga]

Thanks for the report. I think it should treat wait_abandoned specially, because the mutex will be locked, so it should detect wait_abandoned and unlock the mutex before throwing the exception. Otherwise, the client might try to lock() again and it would deadlock.

[anonymous]

void lock() {

unsigned long ret = winapi::wait_for_single_object(m_mtx_hnd, winapi::infinite_time); if(ret != winapi::wait_object_0){

if(ret != winapi::wait_abandoned) {

error_info err = system_error_code(); throw interprocess_exception(err);

} else {

winapi::release_mutex(m_mtx_hnd); throw interprocess_exception(0, "wait_abandoned");

}

}

}

bool try_lock() {

unsigned long ret = winapi::wait_for_single_object(m_mtx_hnd, 0); if(ret == winapi::wait_object_0){

return true;

} else if(ret == winapi::wait_timeout){

return false;

} else if(ret == winapi::wait_abandoned){

winapi::release_mutex(m_mtx_hnd); throw interprocess_exception(0, "wait_abandoned");

} else{

error_info err = system_error_code(); throw interprocess_exception(err);

}

}

bool timed_lock(const boost::posix_time::ptime &abs_time) {

if(abs_time == boost::posix_time::pos_infin){

this->lock(); return true;

}

boost::posix_time::ptime now = microsec_clock::universal_time(); if(now >= abs_time){

return false;

}

unsigned long ret = winapi::wait_for_single_object

(m_mtx_hnd, (abs_time - now).total_milliseconds());

if(ret == winapi::wait_object_0){

return true;

} else if(ret == winapi::wait_timeout){

return false;

} else if(ret == winapi::wait_abandoned){

winapi::release_mutex(m_mtx_hnd); throw interprocess_exception(0, "wait_abandoned");

} else{

error_info err = system_error_code(); throw interprocess_exception(err);

}

}

[Ion Gaztañaga]

(In [86512]) Fixes #9284 ("WaitForSingleObject(mutex) must handle WAIT_ABANDONED")

[megaposer]

The fix is incomplete and should have been applied to all wait_for_single_object API wrapper methods in boost_1_58_0/boost/interprocess/sync/windows/winapi_wrapper_common.hpp:

line 49 : inline bool winapi_wrapper_try_wait_for_single_object(void *handle) ...
line 64 : inline bool winapi_wrapper_timed_wait_for_single_object(void *handle, const boost::posix_time::ptime &abs_time) ...

In both cases, the state WAIT_ABANDONED => winapi::wait_abandoned can occur and is not properly handled.

That means the orphaned synchronization object will be acquired, but the code would throw an exception without previously releasing the synchronization object.

Next time another thread tries to acquire the synchronization object, the object would still be locked. If the same thread tries to acquire the object, it would succeed, but the number of releases would not correctly offset the number of acquisitions.

[megaposer <hp@…>]

Patch to fix remaining synchronization object WAIT_ABANDONDED issues

[Vizor <sid@…>]

Created pull request, that fixes this: ​https://github.com/boostorg/interprocess/pull/51