Inconsistent error: "NoMethodError: undefined method `new' on nil:NilClass." #2124

Closed
presidentbeef opened this Issue Jan 14, 2013 · 7 comments

2 participants

@presidentbeef

I am seeing inconsistent failures when running Brakeman tests on Travis.

$ ruby --version
rubinius 2.0.0.rc1 (1.8.7 release yyyy-mm-dd JI) [i686-pc-linux-gnu]
 1) Error:
test_disable_mass_assignment_by_module(MassAssignDisableTest):
NoMethodError: undefined method `new' on nil:NilClass.
    kernel/delta/kernel.rb:81:in `new (method_missing)'
    /home/travis/builds/presidentbeef/brakeman/test/../lib/brakeman/checks.rb:137:in `run_checks_parallel'
    kernel/bootstrap/thread18.rb:56:in `__run__'

This is the failing line. It is calling new on a variable which is supposed to contain a class. The variable is from an outer scope while the new call is inside a new thread.

Rerunning the job on Travis resolves the issue.

@dbussink
Rubinius member

Are you able to reproduce the problem locally? One thing probably unrelated though, this like is probably not thread safe: https://github.com/presidentbeef/brakeman/blob/master/lib/brakeman/checks.rb#L152. It could be that multiple threads run that code at the same time.

@presidentbeef

I was not able to reproduce locally on OS X. Tried on Linux but couldn't install Rubinius so I gave up.

Line 152 is in the main thread, unless I am missing something.

@dbussink
Rubinius member

Oh, sorry, I read that wrong yeah. I'll see if I can reproduce it then. What was the problem when compiling Rubinius in Linux? It should work pretty easy.

@presidentbeef

I was installing with RVM and had troubles getting libyaml setup. Probably not Rubinius' fault.

@presidentbeef presidentbeef referenced this issue in presidentbeef/brakeman Jan 15, 2013
Merged

Test more rubies #231

@dbussink
Rubinius member

I've been running the test suite a bunch of times on both OS X and Linux with latest master and haven't been able to reproduce the problem. Perhaps it's due to an older version on Travis? Is that the only place where you've seen these errors?

@presidentbeef

I haven't seen the error anywhere else, but then again I do not use Rubinius regularly. If you are running against master, and Travis was using 2.0.0rc1, then perhaps it is fixed?

@dbussink dbussink added a commit that closed this issue Mar 25, 2013
@dbussink dbussink Fix thread safety issue for flushing variable scopes
There was a race condition when flushing the variable scope, because
another thread might already see isolated set to true, before the heap
locals were properly initialized.

This result in this other thread seeing nil as the local value instead
of the proper value. This commit changes the flushing strategy to be
both safe for reads and writes of the locals.

The first step is to delay the allocation of the heap locals tuple. We
then atomically indicate that this scope uses heap locals. If any other
thread also wants to use the scope, it will spin until the heap locals
have been allocated. Spinning for both the read and write ensures that
we don't see any intermediate state during the move from stack to heap
locals. This means no writes are lost and no stale reads occur.

We then create the heap locals and only store then in the heap_locals_
ivar after initializing them, so any reads or writes will see the proper
locals and write to the correct slots.

Fixes #2124
c54998d
@dbussink dbussink closed this in c54998d Mar 25, 2013
@presidentbeef

Sweet, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment