Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: rubinius/rubinius
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 6d6d49edd6f5
Choose a base ref
...
head repository: rubinius/rubinius
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 47d2e9663fb9
Choose a head ref
  • 2 commits
  • 6 files changed
  • 1 contributor

Commits on Jun 2, 2016

  1. Reset logger lock on fork.

    When calling fork(), we hold all running threads to keep them in known states
    across the call to fork().
    
    In the child process, the only thread that persists is the thread that called
    fork(). If another thread was about to write to the logger and got suspended
    before unlocking the logger mutex, the mutex will be locked in the child and
    will block any other thread that tries to lock it. The thread in the parent
    that would have unlocked the lock won't exist in the child.
    
    An alternative would be for the thread calling fork to hold all the relevant
    locks before calling fork(), and then unlocking them all after fork() in the
    child. Functionally, that wouldn't be any different and would impose
    additional costs before the fork().
    
    All code needs to be audited for any locks that could be held across a fork()
    call and all those should be re-init'd in the child.
    brixen committed Jun 2, 2016
    Copy the full SHA
    26df90a View commit details

Commits on Jun 3, 2016

  1. Improved logging of subprocess creation.

    The log will now reliably include the non-core method that invoked the spawn,
    backtick, fork, or exec command. This will point to the application code or
    rubygems code, rather than the Rubinius core library. The location in the app
    code is much more useful typically than the location in the Rubinius core
    library code.
    brixen committed Jun 3, 2016
    Copy the full SHA
    47d2e96 View commit details
Showing with 70 additions and 28 deletions.
  1. +2 −2 machine/builtin/compiled_code.cpp
  2. +42 −26 machine/builtin/system.cpp
  3. +9 −0 machine/logger.cpp
  4. +3 −0 machine/logger.hpp
  5. +13 −0 machine/vm.cpp
  6. +1 −0 machine/vm.hpp
4 changes: 2 additions & 2 deletions machine/builtin/compiled_code.cpp
Original file line number Diff line number Diff line change
@@ -294,8 +294,8 @@ namespace rubinius {
}

bool CompiledCode::core_method(STATE) {
std::string s = file()->cpp_str(state);
if(s.size() >= 5 && strncmp(s.data(), "core/", 5) == 0) return true;
std::string& s = file()->cpp_str(state);
if(s.compare(0, 5, "core/", 5) == 0) return true;
return false;
}

68 changes: 42 additions & 26 deletions machine/builtin/system.cpp
Original file line number Diff line number Diff line change
@@ -393,6 +393,7 @@ namespace rubinius {

int pid = ::fork();

logger::reset_lock();
state->vm()->thread_nexus()->unlock();

if(pid == 0) {
@@ -484,13 +485,17 @@ namespace rubinius {

close(errors[1]);

CallFrame* call_frame = state->vm()->get_ruby_frame(3);

logger::write("spawn: %d: %s, %s, %s:%d",
pid, exe.command(),
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
if(CallFrame* call_frame = state->vm()->get_noncore_frame(state)) {
logger::write("spawn: %d: %s, %s, %s:%d",
pid, exe.command(),
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
} else {
logger::write("spawn: %d: %s, %s",
pid, exe.command(),
state->vm()->name().c_str());
}

int error_no = 0;
ssize_t size;
@@ -600,13 +605,17 @@ namespace rubinius {
close(errors[1]);
close(output[1]);

CallFrame* call_frame = state->vm()->get_ruby_frame(1);

logger::write("backtick: %d: %s, %s, %s:%d",
pid, exe.command(),
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
if(CallFrame* call_frame = state->vm()->get_noncore_frame(state)) {
logger::write("backtick: %d: %s, %s, %s:%d",
pid, exe.command(),
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
} else {
logger::write("backtick: %d: %s, %s",
pid, exe.command(),
state->vm()->name().c_str());
}

int error_no = 0;
ssize_t size;
@@ -680,12 +689,15 @@ namespace rubinius {
*/
ExecCommand exe(state, path, args);

CallFrame* call_frame = state->vm()->get_ruby_frame(3);

logger::write("exec: %s, %s, %s:%d", exe.command(),
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
if(CallFrame* call_frame = state->vm()->get_noncore_frame(state)) {
logger::write("exec: %s, %s, %s:%d", exe.command(),
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
} else {
logger::write("exec: %s, %s", exe.command(),
state->vm()->name().c_str());
}

// From this point, we are serialized.
utilities::thread::SpinLock::LockGuard guard(state->shared().env()->fork_exec_lock());
@@ -825,18 +837,22 @@ namespace rubinius {

int pid = ::fork();

logger::reset_lock();
state->vm()->thread_nexus()->unlock();

if(pid > 0) {
// We're in the parent...
state->shared().machine_threads()->after_fork_parent(state);

CallFrame* call_frame = state->vm()->get_ruby_frame(2);

logger::write("fork: child: %d, %s, %s:%d", pid,
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
if(CallFrame* call_frame = state->vm()->get_noncore_frame(state)) {
logger::write("fork: child: %d, %s, %s:%d", pid,
state->vm()->name().c_str(),
call_frame->file(state)->cpp_str(state).c_str(),
call_frame->line(state));
} else {
logger::write("fork: child: %d, %s", pid,
state->vm()->name().c_str());
}
} else if(pid == 0) {
// We're in the child...
state->vm()->after_fork_child(state);
9 changes: 9 additions & 0 deletions machine/logger.cpp
Original file line number Diff line number Diff line change
@@ -55,6 +55,11 @@ namespace rubinius {
void set_label() {
if(logger_) logger_->set_label();
}

void reset_lock() {
if(logger_) logger_->reset_lock();
}

#define LOGGER_MSG_SIZE 1024

static int append_newline(char* message) {
@@ -255,6 +260,10 @@ namespace rubinius {
}
}

void Logger::reset_lock() {
lock_.init();
}

char* Logger::timestamp() {
time_t clock;

3 changes: 3 additions & 0 deletions machine/logger.hpp
Original file line number Diff line number Diff line change
@@ -60,6 +60,8 @@ namespace rubinius {
void set_label();
void set_loglevel(logger_level level);

void reset_lock();

class Logger {
utilities::thread::SpinLock& lock_;

@@ -84,6 +86,7 @@ namespace rubinius {

virtual void set_label() = 0;

void reset_lock();
char* timestamp();

utilities::thread::SpinLock& lock() {
13 changes: 13 additions & 0 deletions machine/vm.cpp
Original file line number Diff line number Diff line change
@@ -226,6 +226,19 @@ namespace rubinius {
return NULL;
}

CallFrame* VM::get_noncore_frame(STATE) {
for(CallFrame* frame = call_frame_; frame; frame = frame->previous) {
if(frame->native_method_p()) continue;

CompiledCode* code = frame->compiled_code;
if(code && !code->nil_p()) {
if(!code->core_method(state)) return frame;
}
}

return NULL;
}

CallFrame* VM::get_variables_frame(ssize_t up) {
CallFrame* frame = call_frame_;

1 change: 1 addition & 0 deletions machine/vm.hpp
Original file line number Diff line number Diff line change
@@ -277,6 +277,7 @@ namespace rubinius {
CallFrame* get_ruby_frame(ssize_t up=0);
CallFrame* get_variables_frame(ssize_t up=0);
CallFrame* get_scope_frame(ssize_t up=0);
CallFrame* get_noncore_frame(STATE);

bool scope_valid_p(VariableScope* scope);