-
Notifications
You must be signed in to change notification settings - Fork 177
mv iter close fix
- merged to master
- code complete - June 18, 2014
- development started - June 17, 2014
This is an eleveldb branch fixing problems in basho/eleveldb repository.
The primary goal of the mv-tuning7 branch in the basho/eleveldb repository was to fix a segfault problem caused by keeping a C++ class object on the Erlang heap. This goal is achieved in the branch. A secondary goal was to make the database close and interator close APIs asynchronous. The code for the secondary goal was close, but contained a race condition that could lead to new segfaults. The mv-iter-close-fix corrects the asynchronous race conditions and adds a few defensive measures.
The essential changes are in two functions of c_src/refobjects.cc: ErlRefObject::InitiateCloseRequest() and ErlRefObject::RefDec(). ErlRefObject is a base class to the key DbObject and ItrObject classes. These two functions coordinate the destruction of the classes knowing that often there are two threads simultaneously releasing the object for destruction. InitiateCloseRequest() is called by the thread that must wait until all other threads / users of the object are done. It is typically called by a "C" thread, but it could be called by the Erlang garbage collector thread. The last user of the object will call RefDec() which detects it is the last user and executes some special logic to inform InitiateCloseRequest() that all users of the object are done.
The underlying concept of InitiateCloseRequest() and RefDec() was correct. There were some details that were wrong and could lead to two delete calls for the same object. Bad!
-
InitiateCloseRequest() could skip its pthread_cond_wait() call if the thread in RefDec() had decremented the reference counter but not yet entered the mutex protected zone. The result could have InitiateCloseRequest() calling RefDec() at the end of the routine AND the other thread causing another delete through its RefInc()/RefDec() sequence. The change to the "if" statement now verifies both the proper outstanding reference count AND whether the last user thread has completed the pthread_cond_broadcast() call.
-
RefDec() saves the count of references after its atomic decrement. Then it enters a mutex protected zone to send the pthread_cond_broadcast(). The actual reference count needed within mutex protected zone is the reference count at that moment, not the count from sometime in the past (before receiving the mutex).
The following changes reflect incremental work that hardened the code, but did not address the primary problem.
ErlRefObject::ClaimCloseFromCThread() tightened the window for a known race condition that might lead to a segfault read. The condition still exists but now is even harder to reach.