Modify

Opened 4 years ago

Last modified 2 years ago

#9284 reopened Bugs

WaitForSingleObject(mutex) must handle WAIT_ABANDONED

Reported by: huyuguang@… Owned by: Ion Gaztañaga
Milestone: To Be Determined Component: interprocess
Version: Boost 1.58.0 Severity: Problem
Keywords: Cc:

Description

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);

}

Attachments (1)

boost_interprocess_1_58_0.patch (1.8 KB) - added by megaposer <hp@…> 2 years ago.
Patch to fix remaining synchronization object WAIT_ABANDONDED issues

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by anonymous

comment:2 Changed 4 years ago by 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.

comment:3 Changed 4 years ago by 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.

comment:4 Changed 4 years ago by 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);

}

}

comment:5 Changed 4 years ago by Ion Gaztañaga

Resolution: fixed
Status: newclosed

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

comment:6 Changed 2 years ago by megaposer

Resolution: fixed
Status: closedreopened
Version: Boost 1.54.0Boost 1.58.0

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.

Changed 2 years ago by megaposer <hp@…>

Patch to fix remaining synchronization object WAIT_ABANDONDED issues

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The owner will remain Ion Gaztañaga.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.