Skip to content

Commit

Permalink
Preserve ThreadNexus phase_flag state.
Browse files Browse the repository at this point in the history
The ThreadNexus::phase_flag is part of the thread synchronization mechanism
and additionally used to put the system into a sort of 'singled-threaded mode'
that can run arbitrary code. In order to preserve this, we have to know that
when we 'acquired' the lock whether the lock was already held by us. If it
was, we don't want to release it.
brixen committed Jun 9, 2016
1 parent ec8d11c commit 3f7bc1e
Showing 4 changed files with 38 additions and 15 deletions.
18 changes: 12 additions & 6 deletions machine/builtin/system.cpp
Original file line number Diff line number Diff line change
@@ -386,14 +386,16 @@ namespace rubinius {
state->shared().machine_threads()->before_fork_exec(state);
state->memory()->set_interrupt();

state->vm()->thread_nexus()->lock(state->vm());
ThreadNexus::LockStatus status = state->vm()->thread_nexus()->lock(state->vm());

// If execvp() succeeds, we'll read EOF and know.
fcntl(errors_fd, F_SETFD, FD_CLOEXEC);

int pid = ::fork();

state->vm()->thread_nexus()->unlock();
if(status == ThreadNexus::eLocked) {
state->vm()->thread_nexus()->unlock();
}

if(pid == 0) {
// We're in the child...
@@ -721,7 +723,7 @@ namespace rubinius {

state->shared().machine_threads()->before_exec(state);

state->vm()->thread_nexus()->lock(state->vm());
ThreadNexus::LockStatus status = state->vm()->thread_nexus()->lock(state->vm());

void* old_handlers[NSIG];

@@ -762,7 +764,9 @@ namespace rubinius {
sigaction(i, &action, NULL);
}

state->vm()->thread_nexus()->unlock();
if(status == ThreadNexus::eLocked) {
state->vm()->thread_nexus()->unlock();
}

state->shared().machine_threads()->after_exec(state);

@@ -855,11 +859,13 @@ namespace rubinius {
state->shared().machine_threads()->before_fork(state);
state->memory()->set_interrupt();

state->vm()->thread_nexus()->lock(state->vm());
ThreadNexus::LockStatus status = state->vm()->thread_nexus()->lock(state->vm());

int pid = ::fork();

state->vm()->thread_nexus()->unlock();
if(status == ThreadNexus::eLocked) {
state->vm()->thread_nexus()->unlock();
}

if(pid > 0) {
// We're in the parent...
23 changes: 18 additions & 5 deletions machine/thread_nexus.hpp
Original file line number Diff line number Diff line change
@@ -49,6 +49,12 @@ namespace rubinius {
eWaiting = 0x82,
};

enum LockStatus {
eNotLocked = 0x0,
eHeldLock = 0x1,
eLocked = 0x2,
};

const static int cYieldingPhase = 0x80;

ThreadNexus()
@@ -67,6 +73,12 @@ namespace rubinius {
rubinius::bug("attempt to destroy ThreadNexus");
}

private:
LockStatus to_lock_status(bool flag) {
return flag ? eHeldLock : eLocked;
}

public:
std::mutex& fork_mutex() {
return fork_mutex_;
}
@@ -119,7 +131,7 @@ namespace rubinius {

bool waiting_lock(VM* vm);

bool try_lock(VM* vm) {
LockStatus try_lock(VM* vm) {
while(stop_p()) {
bool held = waiting_lock(vm);

@@ -128,7 +140,7 @@ namespace rubinius {
if(try_checkpoint(vm)) {
if(stop_p()) {
unset_stop();
return true;
return to_lock_status(held);
}
}
}
@@ -137,14 +149,15 @@ namespace rubinius {
if(!held) unlock();
}

return false;
return eNotLocked;
}

void lock(VM* vm) {
waiting_lock(vm);
LockStatus lock(VM* vm) {
bool held = waiting_lock(vm);
set_stop();
checkpoint(vm);
unset_stop();
return to_lock_status(held);
}

void unlock() {
7 changes: 5 additions & 2 deletions machine/thread_phase.hpp
Original file line number Diff line number Diff line change
@@ -14,16 +14,19 @@ namespace rubinius {
*/
class LockPhase {
State* state_;
ThreadNexus::LockStatus status_;

public:
LockPhase(STATE)
: state_(state)
{
state->vm()->thread_nexus()->lock(state->vm());
status_ = state->vm()->thread_nexus()->lock(state->vm());
}

~LockPhase() {
state_->vm()->thread_nexus()->unlock();
if(status_ == ThreadNexus::eLocked) {
state_->vm()->thread_nexus()->unlock();
}
}
};

5 changes: 3 additions & 2 deletions machine/vm.hpp
Original file line number Diff line number Diff line change
@@ -436,12 +436,13 @@ namespace rubinius {
void checkpoint(STATE) {
metrics().machine.checkpoints++;

if(thread_nexus_->try_lock(this)) {
ThreadNexus::LockStatus status = thread_nexus_->try_lock(this);
if(status != ThreadNexus::eNotLocked) {
metrics().machine.stops++;

collect_maybe(state);

thread_nexus_->unlock();
if(status == ThreadNexus::eLocked) thread_nexus_->unlock();
}

if(profile_counter_++ >= profile_interval_) {

0 comments on commit 3f7bc1e

Please sign in to comment.