C function thread safety in Ruby 1.8

In ruby 1.8 (I’m speaking only of MRI here) you don’t have to worry about threads being context switched in the middle of (most) C functions. 1.8 switches threads in really only 1 way: a call to rb_thread_schedule(). This function is called from various other ruby internal functions and macros (CHECK_INTS) but as long you aren’t hitting any of those in the C function in question you won’t be switched.

You may know about the infamous SIGVTALRM that ruby sends to itself to schedule threads every 10ms. But all that signal handler does is set a global flag saying that it’s time to switch. CHECK_INTS uses the flag to determine whether or not to call rb_thread_schedule(). So that signal by itself does not actually switch threads.

signal(SIGVTALRM, catch_timer);
...
void catch_timer(int sig)
{
    if (!rb_thread_critical) {
        rb_thread_pending = 1;
    }
}
...
#define CHECK_INTS ...  if (rb_thread_pending) rb_thread_schedule(); ...

Now you DO have to worry about your C function being context switched if you are calling back to methods on ruby objects from your C function. rb_call() calls CHECK_INTS once every 256 times. Another way to get switched is dealing with ruby’s IO functions or rb_thread_select(). But if your C function is primarily self contained then you can rest safe that it will block the whole interpreter until it returns :).

This is one of if not the only reason that ruby’s primitive data structure operations (Array, Hash, String, etc) ARE thread-safe. Because they are implemented in C. If you were to reimplement all of Array, Hash, and Strings methods in ruby by directly translating the C code into ruby you’d probably end up with non thread safe methods.

Even though 1.9 uses native threads I believe it acts in a similar matter, but that’s for another post.

And if someone knows more than me and I misrepresented something, please let me know!

||= is NOT thread safe, neither is Hash.new {|h,k| … }

||= is an extremely useful ruby operator that lots of people learn to love and use often. The problem is it doesn’t mix well with threading (shared writable variables in specific) because it’s not atomic. I’ve been noticing a lot of people making these mistakes recently, even in code which touts being thread safe (even in my own x_x).

It’s an easy mistake to make so I figured I should explain what’s wrong with it and how to avoid it. Let’s take a simple example:

class X
  def lock
    @mutex ||= Mutex.new
    @mutex.synchronize { yield }
  end
end

lock uses ||= to lazily initialize the @mutex instance var, which is normally fine, except in this case. First you need to remember/realize what ||= actually expands to:

@mutex or @mutex = Mutex.new

Ok so let’s assume two threads call the lock method at the “same time”. Here’s the scenario which demonstrates how it’s not thread safe. {{}} represents what the interpreter is currently evaluating:

# >> thread 1
{{@mutex}} or @mutex = Mutex.new
# >> thread 2 (context switch)
{{@mutex}} or @mutex = Mutex.new
# At this point, both threads evaluated @mutex to nil, so they will both go ahead with the assignment
# >> thread 1 (context switch)
@mutex or {{@mutex = Mutex.new
@mutex.synchronize}} { yield }
# thread 1 is already referencing the object stored in @mutex, ready to call synchronize on it, so if another thread changes it, it won't make a difference to thread 1
# >> thread 2 (context switch)
@mutex or {{@mutex = Mutex.new}}
# thread 2 has now "won" the race and set @mutex to the Mutex object it created.  BUT both threads
# will have be acting on different Mutexes, rendering their synchronization useless.

So usually in cases where you want to use the ||= operator with a shared variable you want to actually synchronize the ||= operation with another “more global” mutex. Or in this situation just setting @mutex = Mutex.new in the initialize method would be best. Here’s an example with the global mutex.

class X
  @@class_mutex = Mutex.new
  def lock
    @@class_mutex.synchronize { @mutex ||= Mutex.new }
    @mutex.synchronize { ... }
    # or
    @@class_mutex.synchronize { @mutex ||= Mutex.new }.synchronize { ... }
  end
end

Along the same lines a Hash with a default proc can run you into thread safety problems. Here’s an example:

h = Hash.new{|h,k| sleep Thread.current[:sleep].to_i; h[k] = Mutex.new }
t1 = Thread.new { Thread.current[:sleep] = 1; h[1] }
t2 = Thread.new { h[1] }
p t2.value # => #<Mutex:0xb7c67840>
p t1.value # => #<Mutex:0xb7c63614>
p h[1] # => #<Mutex:0xb7c63614>

The default proc will be called twice, two Mutexes will be created and you could end up in the same situation as the ||= example. The bottom line here is that when you use a default proc in a Hash or the ||= operator, you expect that the variable or hash key in question will only be set a single time. When dealing with threads this isn’t always the case.

How to get a list of un-pushed commits in git

Assuming your remote repository is named origin and you’re dealing with the master branch:

$ git log master ^origin/master

This shows what commits are in master but not in origin/master (which is the remote branch).

This same syntax can be used to see the difference between two local branches, but will show cherry-picked commits as differences which can be confusing:

$ git log master ^production

Determining how much memory a require uses

Here’s a quick little monkeypatch of the require method which will print out a tree showing how much memory each required file used:

# mem.rb
$require_level = 0
module MemTrackingRequire
  def require(name)
    $require_level += 1
    mem = lambda { GC.start; `ps -o rss= -p #$$`.to_i }
    before = mem.call

    required = super
  ensure
    after = mem.call
    $require_level -= 1
    if required
      puts "#{" "*$require_level}#{name} #{after - before}"
      puts "-------"  if $require_level.zero?
    end
  end
end

Object.class_eval { include MemTrackingRequire }

Here are some examples of how to use it (assuming it’s saved as mem.rb)

$ ruby -rmem -e 'require "rubygems"'
  rubygems/rubygems_version 0
  rubygems/defaults 0
   thread.so 24
  thread 28
  rbconfig 36

...

  rubygems/config_file 928
   rubygems 0
  rubygems/custom_require 0
 rubygems 1788  # <-- this is the number you are looking for, it represents the total amount of memory used by requiring 'rubygems'

$ ruby -rubygems -rmem -e 'gem "activerecord", "=2.2.2"; require "activerecord"'
...
   active_record/i18n_interpolation_deprecation 0
  active_record 5896
 activerecord 5896

$ ruby -rmem -e 'require "net/http"'
...
   uri/mailto 0
  uri 60
 net/http 800

Ruby 1.8 define_method scope bug

Update: Patch accepted, bug fixed!

While writing some tests for my tunnel_splitter project I ran into this really weird bug. It’s hard to believe no one has run into this before. I’ll give you the isolated version (which took considerable time to narrow down).

a = 1
Object.send :define_method, :x do
  lambda do  # lambda is necessary to reproduce bug
    p a # => 1
    a = 2
    p a # => 2
  end.call
end
x(nil) # passing at least one arg is necessary to reproduce bug
p a # => 1

This is wrong. Instead of 1, 2, 1, the output should be 1, 2, 2.

Here’s another reproduction, closer to the actual way I encountered the bug. This one is sensitive to the timing of when threads are scheduled. By commenting out the p :blah line you can actually prevent the bug from happening. That makes this bug way more evil and caused me considerable pain.

def callblock; yield; end

a = 1
Object.send :define_method, :x do
  p :blah # comment this line out and things will work as expected
  begin
    callblock do
      raise 'blah'
    end
  rescue
    p a
    a = 2
    p a
  end
end
Thread.new{ x(nil) }.join
p a # => 1

Offender #1
eval.c:8626 proc_invoke()

    if (_block.frame.argc && DMETHOD_P()) {
        NEWOBJ(scope, struct SCOPE);
        OBJSETUP(scope, tmp, T_SCOPE);
        scope->local_tbl = _block.scope->local_tbl;
        scope->local_vars = _block.scope->local_vars;
        scope->flags |= SCOPE_CLONE;
        _block.scope = scope;
    }

If the block being invoked was a block passed to define_method (DMETHOD_P()) and it is being invoked with some number of args other than 0 (_block.frame.argc) then Ruby will “clone” the scope. Effectively, this doesn’t really do much. It creates a new scope struct, but then it points the local vars tables to the exact same place as the original, meaning if you modify a local in this scope, it will be reflected in the original scope. The issue is that it loses the flags of the original scope. We will see why this is important in a second.

Offender #2
eval.c:8214

static void
scope_dup(scope)
    struct SCOPE *scope;
{
    ID *tbl;
    VALUE *vars;

    scope->flags |= SCOPE_DONT_RECYCLE;
    if (scope->flags & SCOPE_MALLOC) return;

    if (scope->local_tbl) {
        tbl = scope->local_tbl;
        vars = ALLOC_N(VALUE, tbl[0]+1);
        *vars++ = scope->local_vars[-1];
        MEMCPY(vars, scope->local_vars, VALUE, tbl[0]);
        scope->local_vars = vars;
        scope->flags |= SCOPE_MALLOC;
    }
}

scope_dup will copy the local_vars (local variables’ values) table into a new memory address. Whenever this is done, changes to the variables in this scope will NOT affect the original scope’s variables (unless they are explicitly copied/pointed back). Note that scope_dup will return right away and do nothing if the scope has the SCOPE_MALLOC flag set, which scope_dup sets after duping a scope, effectively making it perform only one dup per scope, even if called multiple times.

Offender #1 + Offender #2 = fail

Each of these pieces of code by themselves will not cause the bug to occur. But put them together, specifically the scope cloning conditional and then the scope_dup function, and voila, your weekend has become an MRI debug session.

The issue is that the proc_invoke() scope cloning conditional will not copy the parent scope’s SCOPE_MALLOC flag. Which means if you call scope_dup on a cloned scope you will end up duping the local variables even if they had already been duped once before. Essentially you will end up duping a scope’s local variables twice, which means the parent scope’s variables will not be affected at all by changes to the current scope’s local variables.

Solution

If we just copy the SCOPE_MALLOC flag when cloning a scope all our problems are solved:

      ...
        scope->local_vars = _block.scope->local_vars;
        scope->flags |= SCOPE_CLONE;
        scope->flags |= (_block.scope->flags & SCOPE_MALLOC);  // add this line
        _block.scope = scope;
    }

This way if a duped scope is cloned then duped, the local var tables will still be pointing to the same place.

By itself this actually causes ruby to segfault due to multiple free()s on the same memory.

gc.c:1271

            if (!(RANY(obj)->as.scope.flags & SCOPE_CLONE) && vars[0] == 0)
                RUBY_CRITICAL(free(RANY(obj)->as.scope.local_tbl));
            if (RANY(obj)->as.scope.flags & SCOPE_MALLOC))
                RUBY_CRITICAL(free(vars)); // double free segfault here!

We can modify the second conditional to make sure the scope doesn’t have the SCOPE_CLONE flag before free()ing.

            if (!(RANY(obj)->as.scope.flags & SCOPE_CLONE) && vars[0] == 0)
                RUBY_CRITICAL(free(RANY(obj)->as.scope.local_tbl));
            if (RANY(obj)->as.scope.flags & SCOPE_MALLOC)) && !(RANY(obj)->as.scope.flags & SCOPE_CLONE))
                RUBY_CRITICAL(free(vars));

Conclusion

I’m not sure if this is the best solution. I’m not even sure what the scope cloning conditional is needed for. I can’t think of a reason why you would need to make some modification to the scope of a block passed to define_method called with 1 or more arguments, which you wouldn’t need to make if it were called with 0 arguments. There probably is some reason though, and it’d be nice if someone could point out what it is. Also I’m not sure if it might be better to just copy all the scope flags instead of just the SCOPE_MALLOC one.

Also, I’m definitely not an expert on the internals of MRI so it’s possible this solution actually causes some other bugs.

Here’s the patch

1.8.7 and 1.9

This bug also occurs in 1.8.7 but not in 1.9.

bug filed

Protecting your ~/.gem directory

If you accidentally run gem install without sudo you’ll end up with the gem installed into your ~/.gem directory instead of your system gem path. This can cause annoyances, one of which I mentioned here.

To protect yourself from your own stupidity just deny yourself access to the directory:

chmod -R u-w ~/.gem

Now try installing a gem without sudo and you’ll get:

$ gem install hpricot
WARNING:  Installing to ~/.gem since /usr/lib/ruby/gems/1.8 and
	  /usr/bin aren't both writable.
ERROR:  While executing gem ... (Gem::FilePermissionError)
    You don't have write permissions into the /home/me/.gem/ruby/1.8 directory.

This has already saved me a few times. Thanks to Arild Shirazi for the suggestion.

How and when anonymous Modules get named

How are the names of these anonymous Modules being set? Looks like some assignment magic, right?

m1 = Module.new  # => #<Module:0xb7c....>
A = m1                 # => A
m1                       # => A
m2 = Module.new  # => #<Module:0xb7d....>
B = C = m2           # => B
C                          # => B
C = m2                 # => B
m2                        # => B

The rule ruby is following here has nothing to do with assignment. It has to do with object inspection. Specifically calling to_s or inspect (which calls to_s) on the Module. (IRB calls inspect on the object returned from your statement)

If a Module is unnamed, calling to_s on it will cause ruby to look through all constants and Modules (and the instance variables of all Modules) until it finds one that equals our unnamed Module. If one matches it sets the name of the Module to that constant. If nothing matches the Module stays unnamed.

Try adding 1 million constants and then calling to_s on an anonymous Module. It’ll be noticeably slower:

require 'benchmark'

Benchmark.realtime { Module.new.to_s }
# => 0.000191926956176758

(0..1000000).each {|n| Object.const_set("Const#{n}", n) }

Benchmark.realtime { Module.new.to_s }
# => 0.1439208984375

Once a constant is found the result is memoized and not looked up again. We can show this and prove that the name is not set on assignment with the following code. As long as we never call to_s on the Module while there is a matching constant, the name is never set. We can call to_s after the constant has been removed and the Module will remain unnamed.

m1 = Module.new
A = m1; 0                                # => 0
Object.send :remove_const, :A
m1                                          # => #<Module:0xabc....>

Next we will remove the constant after it has been matched and the name memoized into the Module. You can see the Module still retains the name after the constant is gone.

m2 = Module.new
B = m2 # => B
Object.send :remove_const, :B
m2     # => B
B      # => NameError: uninitialized constant B

This is the function which is eventually called by to_s on an unnamed Module and is responsible for finding a matching constant:

static VALUE
find_class_path(klass)
    VALUE klass;
{
    struct fc_result arg;

    arg.name = 0;
    arg.path = 0;
    arg.klass = klass;
    arg.track = rb_cObject;
    arg.prev = 0;
    if (RCLASS(rb_cObject)->iv_tbl) {
        st_foreach_safe(RCLASS(rb_cObject)->iv_tbl, fc_i, (st_data_t)&arg);
    }
    if (arg.path == 0) {
        st_foreach(rb_class_tbl, fc_i, (st_data_t)&arg);
    }
    if (arg.path) {
        if (!ROBJECT(klass)->iv_tbl) {
            ROBJECT(klass)->iv_tbl = st_init_numtable();
        }
        st_insert(ROBJECT(klass)->iv_tbl, classpath, arg.path);
        st_delete(RCLASS(klass)->iv_tbl, &tmp_classpath, 0);
        return arg.path;
    }
    return Qnil;
}

First, st_foreach_safe will recursively walk down through all of Object’s constants and their descendant constants. If no matching constant is found there it will look through the ruby class table. If a matching constant is found in either of these it will set its __classpath__ (name) to that of the matching constant.

RTunnel 0.4.0 at Google

RTunnel is a client/server which reverse tunnels TCP connections through a firewall or NAT. It achieves the same thing as ssh -R but is more flexible and solves some of the annoyances of ssh.

Version 0.4.0 has been changed from a mutli threaded to event driven architecture using EventMachine. It also now supports public key authentication to restrict access to the server and message digests to prevent tunnel injection attacks, as well as allowing you to restrict the server’s listen ports to a certain range. This is all thanks to Victor Costan whose huge contribution got RTunnel in the door at Google.

Get the latest version of RTunnel on my github. And be sure to check out Victor’s blog and github.

Ridiculous ruby meta programming hack

Ruby 1.8.6 has a bug where Dir.glob will glob on a tainted string in $SAFE level 3 or 4 without raising a SecurityError as would be expected. You can see this from the following code:

lambda { $SAFE = 4; Dir.glob('/**') }.call # raises SecurityError
lambda { $SAFE = 4; Dir.glob(['/**']) }.call # => [ ... files ... ]

So I set out to fix this with pure ruby… and it ended up requiring some really crazy stuff. I’ll first show what I ended up with, then go through and explain why:

class Dir
  class << self
    safe_level_password = File.open('/dev/urandom','r'){|f| f.read(10000) }

    m = Dir.method(:glob)
    define_method :glob do |password, safe, *args|
      raise SecurityError if safe_level_password != password
      $SAFE = safe
      args.flatten!

      # pass along glob opts
      opts = args.last.is_a?(Fixnum) ? args.pop : []

      args.flatten.map do |arg|
        m.call(arg, *opts)
      end.flatten
    end

    eval %{
      alias safe_glob glob
      def glob(*a)
        safe_glob(#{safe_level_password.inspect}, $SAFE, *a)
      end
    }
  end
end

# freeze Dir so that no one can redefine safe_glob* to catch password
Dir.freeze

So first things first. The simple way to fix this bug is to alias glob, iterate over the array passed to the new glob and then call the original glob with one argument at a time, since we know it correctly checks taint with only one argument.

But wait, if we alias the original method then someone could still access the original and pass it an array. So we have to use a “secure” version of alias method chaining. Essentially, we capture the method object of the original method in the local scope, then we use define_method to overwrite the original method name with our new implementation and call the original method object which we have ‘closed’ over. This allows us access to the original method while preventing anyone else from doing so.

But there’s another problem. $SAFE levels are captured in blocks just as local variables. This means our define_method version of glob will always run at the $SAFE level it was defined in, namely $SAFE level 0. Meaning if you call Dir.glob from a $SAFE level 4 it will still get executed at level 0. This is of course the exact opposite of what we want. We are in a worse position now than before. Now we could call Dir.glob with a single tainted parameter and it would work.

How do we fix this? We need to use def to define the method so that the current $SAFE level 0 isn’t captured. Instead $SAFE will reflect the $SAFE level of the caller. But if we use def we can’t use the secure alias method technique.

One option is to have the define_method version set the $SAFE level explicitly before calling glob. But then we run into the issue of how to know what to set it to? There is no way of determining the $SAFE level of the caller without explicitly passing it in.

Ok, so what if we def a method which then calls the define_method method and passes its $SAFE level as an argument? Well then the problem is how do you give the def method access to the define_method method without giving other evil code access to the define_method method as well. Because then that evil code could just lie and pass a level of 0.

This is the crazy part. The way to prevent the evil method from executing the define_method method is to use a password!

Both the def and define_method methods can share a secret which is only available in the local scope where they are defined. Since the password is only a string we can use eval to create the def method and insert the password into it. As long as the define_method method verifies the secret is correct before continuing we can be sure the only method able to call it is the def method.

I never thought I’d be sharing a secret between methods. I know this is a big house of cards. Can anyone figure out how to make it tumble? Or a better way to fix the glob bug (without C).

And yes, you must also do the same for Dir.[] but I left it out for sake of brevity.

BTW, here’s the patch if you actually care enough to recompile:

--- dir.c.orig	2009-02-21 21:49:09.000000000
+++ dir.c	2009-02-21 21:49:38.000000000
@@ -1659,7 +1659,7 @@
     for (i = 0; i < argc; ++i) {
 	int status;
 	VALUE str = argv[i];
-	StringValue(str);
+	SafeStringValue(str);
 	status = push_glob(ary, RSTRING(str)->ptr, flags);
 	if (status) GLOB_JUMP_TAG(status);
     }

Preventing Frame Busting and Click Jacking (UI Redressing)

Some websites are under the impression this very old frame busting code can prevent click jacking attacks:

try {
  if (top.location.hostname != self.location.hostname) throw 1;
} catch (e) {
  top.location.href = self.location.href;
}

Here’s a very simple way around this which works in both FF and IE7: (update, a way to work around this prevetion here)

  var prevent_bust = 0
  window.onbeforeunload = function() { prevent_bust++ }
  setInterval(function() {
    if (prevent_bust > 0) {
      prevent_bust -= 2
      window.top.location = 'http://server-which-responds-with-204.com'
    }
  }, 1)

The server only needs to respond with:

HTTP/1.1 204 No Content

On most browsers a 204 (No Content) HTTP response will do nothing, meaning it will leave you on the current page. But the request attempt will override the previous frame busting attempt, rendering it useless. If the server responds quickly this will be almost invisible to the user.

Update: If the frame busting code is at the beginning of the page, before any content loads, then even though the frame busting will be prevented, so will the loading of the remainder of the page. This means that your content would be hidden and un-clickjackable (only in FF, see below for IE).

So what can a website do to prevent clickjacking? I’m not a security expert but this seems to cover almost all the cases:

First, have your page load with all content hidden using CSS. Something along the lines of:

<body style="display:none" ...>

Then use some variant of the frame busting code, but instead of busting, use it to determine whether or not to display your content:

try {
  if (top.location.hostname != self.location.hostname)
    throw 1;

  document.body.style.display = 'block';
} catch (e) {
  // possible clickjack attack, leave content hidden
}

This covers most of the cases. It covers IE’s SECURITY=RESTRICTED which allows you to turn off scripting for an iframe. If your site is loaded like this, your script will not run and your content will remain hidden (as mentioned here). And it covers a standard clickjack attack by not displaying your content if it detects that it has been framed. What it doesn’t cover is a user who comes to your site with javascript disabled (who will see nothing). You of course could present them with a message saying javascript is required (using <noscript>). Sucks, but it seems at this point that is the price to pay for clickjacking protection.

If you have or know of a better solution please let me know.

Note to users: NoScript can protect you from clicking on invisible elements.