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: 06822ff0764a
Choose a base ref
...
head repository: rubinius/rubinius
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 415b1c8aa3c1
Choose a head ref
  • 8 commits
  • 32 files changed
  • 3 contributors

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
  2. Copy the full SHA
    949b25e View commit details

Commits on Jun 4, 2016

  1. Reworked running finalizers.

    brixen committed Jun 4, 2016
    Copy the full SHA
    2310fd3 View commit details
  2. Copy the full SHA
    fca58e9 View commit details
  3. Merge pull request #3659 from rubinius/finalizers

    Reworked finalizer mechanism.
    brixen committed Jun 4, 2016
    Copy the full SHA
    26403b2 View commit details
  4. Copy the full SHA
    ba08476 View commit details
  5. Removed double finalization bandaid.

    We need to ensure this is fixed now.
    brixen committed Jun 4, 2016
    Copy the full SHA
    415b1c8 View commit details
4 changes: 0 additions & 4 deletions core/loader.rb
Original file line number Diff line number Diff line change
@@ -758,10 +758,6 @@ def epilogue

run_at_exits

@stage = "running object finalizers"
::GC.start
ObjectSpace.run_finalizers

flush_stdio

rescue Object => e
11 changes: 8 additions & 3 deletions core/object_space.rb
Original file line number Diff line number Diff line change
@@ -52,6 +52,14 @@ def self.each_object(what=nil, &block)
def self.define_finalizer(obj, prc=nil, &block)
prc ||= block

if obj.kind_of? ImmediateValue or obj.kind_of? Float
raise ArgumentError, "can't define finalizer for #{obj.class}"
end

if obj.frozen?
raise RuntimeError, "can't modify frozen #{obj.class}"
end

if obj.equal? prc
# This is allowed. This is the Rubinius specific API that calls
# __finalize__ when the object is finalized.
@@ -74,9 +82,6 @@ def self.undefine_finalizer(obj)
return obj
end

def self.run_finalizers
end

def self.garbage_collect
GC.start
end
2 changes: 0 additions & 2 deletions core/process.rb
Original file line number Diff line number Diff line change
@@ -83,8 +83,6 @@ def self.fork
end
end

ObjectSpace.run_finalizers

# Do not use Kernel.exit. This raises a SystemExit exception, which
# will run ensure blocks. This is not what MRI does and causes bugs
# in programs. See issue http://github.com/rubinius/rubinius/issues#issue/289 for
2 changes: 1 addition & 1 deletion machine/builtin/call_site.hpp
Original file line number Diff line number Diff line change
@@ -181,7 +181,7 @@ namespace rubinius {
cache->name(name);
cache->ip(ip);

state->memory()->needs_finalization(state, cache,
state->memory()->native_finalizer(state, cache,
(memory::FinalizerFunction)&CallSite::finalize);

state->vm()->metrics().machine.call_site_count++;
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;
}

4 changes: 2 additions & 2 deletions machine/builtin/data.cpp
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ namespace rubinius {
data->internal(rdata);

if(mark || free) {
state->memory()->needs_finalization(state, data,
state->memory()->extension_finalizer(state, data,
(memory::FinalizerFunction)&Data::finalize);
}

@@ -73,7 +73,7 @@ namespace rubinius {
data->internal(rdata);

if(type->function.dmark || type->function.dfree) {
state->memory()->needs_finalization(state, data,
state->memory()->extension_finalizer(state, data,
(memory::FinalizerFunction)&Data::finalize);
}

5 changes: 2 additions & 3 deletions machine/builtin/dir.cpp
Original file line number Diff line number Diff line change
@@ -18,9 +18,8 @@ namespace rubinius {
Dir* d = Dir::allocate(state, G(dir));
d->os(0);

state->memory()->needs_finalization(state, d,
(memory::FinalizerFunction)&Dir::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, d,
(memory::FinalizerFunction)&Dir::finalize);

return d;
}
5 changes: 2 additions & 3 deletions machine/builtin/encoding.cpp
Original file line number Diff line number Diff line change
@@ -729,9 +729,8 @@ namespace rubinius {

c->converter(NULL);

state->memory()->needs_finalization(state, c,
(memory::FinalizerFunction)&Converter::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, c,
(memory::FinalizerFunction)&Converter::finalize);

return c;
}
7 changes: 3 additions & 4 deletions machine/builtin/ffi_pointer.cpp
Original file line number Diff line number Diff line change
@@ -115,14 +115,13 @@ namespace rubinius {

if(autorelease) {
if(!set_finalizer) {
state->memory()->needs_finalization(state, this,
(memory::FinalizerFunction)&Pointer::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, this,
(memory::FinalizerFunction)&Pointer::finalize);
set_finalizer = true;
}
} else {
if(set_finalizer) {
state->memory()->set_ruby_finalizer(this, cNil);
state->memory()->managed_finalizer(state, this, cNil);
set_finalizer = false;
}
}
10 changes: 4 additions & 6 deletions machine/builtin/fiber.cpp
Original file line number Diff line number Diff line change
@@ -38,9 +38,8 @@ namespace rubinius {
fib->data(state->vm()->new_fiber_data(true, fib->stack_size()->to_native()));
fib->data()->set_call_frame(state->vm()->call_frame());

state->memory()->needs_finalization(state, fib,
(memory::FinalizerFunction)&Fiber::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, fib,
(memory::FinalizerFunction)&Fiber::finalize);

state->vm()->current_fiber.set(fib);
state->vm()->root_fiber.set(fib);
@@ -119,9 +118,8 @@ namespace rubinius {

state->vm()->metrics().system.fibers_created++;

state->memory()->needs_finalization(state, fib,
(memory::FinalizerFunction)&Fiber::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, fib,
(memory::FinalizerFunction)&Fiber::finalize);

return fib;
#else
10 changes: 4 additions & 6 deletions machine/builtin/fsevent.cpp
Original file line number Diff line number Diff line change
@@ -34,9 +34,8 @@ namespace rubinius {
if(fsevent->kq() < 0) {
logger::error("%s: unable to create kqueue", strerror(errno));
} else {
state->memory()->needs_finalization(state, fsevent,
(memory::FinalizerFunction)&FSEvent::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, fsevent,
(memory::FinalizerFunction)&FSEvent::finalize);
}

return fsevent;
@@ -84,9 +83,8 @@ namespace rubinius {
if(fsevent->in() < 0) {
logger::error("%s: unable to create inotify", strerror(errno));
} else {
state->memory()->needs_finalization(state, fsevent,
(memory::FinalizerFunction)&FSEvent::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, fsevent,
(memory::FinalizerFunction)&FSEvent::finalize);
}

return fsevent;
70 changes: 43 additions & 27 deletions machine/builtin/system.cpp
Original file line number Diff line number Diff line change
@@ -396,6 +396,7 @@ namespace rubinius {

int pid = ::fork();

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

if(pid == 0) {
@@ -487,13 +488,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;
@@ -603,13 +608,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;
@@ -683,12 +692,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());
@@ -828,18 +840,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);
@@ -1497,7 +1513,7 @@ namespace rubinius {

Object* System::vm_set_finalizer(STATE, Object* obj, Object* fin) {
if(!obj->reference_p()) return cFalse;
state->memory()->set_ruby_finalizer(obj, fin);
state->memory()->managed_finalizer(state, obj, fin);
return cTrue;
}

5 changes: 2 additions & 3 deletions machine/builtin/thread.cpp
Original file line number Diff line number Diff line change
@@ -90,9 +90,8 @@ namespace rubinius {

thr->function(function);

state->memory()->needs_finalization(state, thr,
(memory::FinalizerFunction)&Thread::finalize,
memory::FinalizeObject::eUnmanaged);
state->memory()->native_finalizer(state, thr,
(memory::FinalizerFunction)&Thread::finalize);

state->vm()->metrics().system.threads_created++;

2 changes: 0 additions & 2 deletions machine/capi/io.cpp
Original file line number Diff line number Diff line change
@@ -126,8 +126,6 @@ namespace rubinius {

RIO* rio = as_.rio;

if(!rio->f) return true; // might be masking bug where finalizer called twice

if(rio->finalize) rio->finalize(rio, true);

bool ok = (fclose(rio->f) == 0);
2 changes: 1 addition & 1 deletion machine/environment.cpp
Original file line number Diff line number Diff line change
@@ -551,7 +551,7 @@ namespace rubinius {

shared->thread_nexus()->lock(state->vm());

shared->finalizer_handler()->finish(state);
shared->finalizer()->finish(state);

NativeMethod::cleanup_thread(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() {
2 changes: 1 addition & 1 deletion machine/machine_threads.cpp
Original file line number Diff line number Diff line change
@@ -15,8 +15,8 @@ namespace rubinius {

MachineThread::MachineThread(STATE, std::string name, StackSize stack_size)
: vm_(state->shared().thread_nexus()->new_vm(&state->shared(), name.c_str()))
, thread_running_(false)
, stack_size_(stack_size)
, thread_running_(false)
, thread_exit_(false)
{
state->shared().machine_threads()->register_thread(this);
5 changes: 3 additions & 2 deletions machine/machine_threads.hpp
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@

#include "util/thread.hpp"

#include <atomic>
#include <string>
#include <list>

@@ -16,12 +17,12 @@ namespace rubinius {

class MachineThread {
VM* vm_;
bool thread_running_;
uint32_t stack_size_;

protected:

bool thread_exit_;
std::atomic<bool> thread_running_;
std::atomic<bool> thread_exit_;

public:

Loading