pluginaweek / state_machine

Adds support for creating state machines for attributes on any Ruby class
http://www.pluginaweek.org
MIT License
3.74k stars 507 forks source link

Optional Thread-safe Transitioning #307

Closed jemc closed 9 years ago

jemc commented 10 years ago

So, I realize that state_machine makes no claim to being thread-safe, and I realize that implementing thread-aware protections into state_machine is not a trivial task, especially if you wanted the feature to be comprehensive enough to actually start claiming to be thread-safe. I also realize that such a feature could actually have a negative performance impact on those who are only accessing their Machines from one thread and therefore have no use for this feature. So I'm no asking for it to be comprehensive, and I'm not asking for it to be mandatory. I'm hoping to get some kind of opt-in thread protection specifically for transitions.

To be clear, what I am asking for is the option to make the Machine helpers that event creates (https://github.com/pluginaweek/state_machine/blob/master/lib/state_machine/event.rb#L236-L254) wrapped in a Machine-level Mutex (or Monitor, if they need to be re-entrant). While this may not solve all of the possible issues you may have while accessing a Machine from multiple threads, it solves the problems I have with my use case.

If you don't want to include that opt-in mechanism, I'd ask for some documented API by which I could make this happen in my code without monkey-patching your internals (or having to wrap the Machine in a delegator object).

Let me know what your thoughts are on this.

Here is my current monkey-patched hack I've been playing with:

require 'state_machine'

class StateMachine::Machine
  __orig = instance_method :define_helper

  define_method :define_helper do |scope, meth, *args, &block|
    Thread.exclusive { @__machine_lock ||= Monitor.new }

    # TODO: handle cases where block is nil
    #   and meth is a string to eval

    # TODO: evaluate whether some helpers can
    #  remain lock-naked and unprotected

    new_block = Proc.new do |*a,&b| 
      @__machine_lock.synchronize do
        block.call *a,&b
      end
    end if block

    __orig.bind(self).call scope, meth, *args, &new_block
  end
end
class Vehicle
  state_machine :state, :initial => :parked do

    before_transition do
      puts "a transition is occurring"
      sleep 0.25 # to simulate an expensive operation
    end

    around_transition do |block|
      sleep 0.25 # to simulate an expensive operation
      block.call
      sleep 0.25 # to simulate an expensive operation
    end

    after_transition do
      puts "a transition occurred"
      sleep 0.25 # to simulate an expensive operation
    end

    event :foo do
      transition :parked => :idling
    end

    event :bar do
      transition :parked => :first_gear
    end
  end
end
a = Vehicle.new

Thread.abort_on_exception = true
threads = []

100.times { |i|
  threads << Thread.new {
    if (Random.rand*2).to_i.odd?
      puts 'foo returned true' if a.foo
    else
      puts 'bar returned true' if a.bar
    end
  }
}

threads.each &:join
sdbondi commented 9 years ago

This has got me wondering if some weird problems I'm having is with the lack of thread safety in state machine. Currently I'm running it in production with Rails 4 (i.e thread_safe!). :fearful:

jemc commented 9 years ago

@sdbondi - yeah, this gem is most certainly not thread safe :wink:. I ended up using other gems, though, so I don't care much about this issue ticket anymore.

sdbondi commented 9 years ago

@jemc Are you using your hegemon gem?

jemc commented 9 years ago

Actually, in the end I ended up using micromachine and a Mutex. :wink:

sdbondi commented 9 years ago

@jemc Thanks :+1: The state changes currently occur in my multi threaded workers so I've put a class level Mutex around the operations and added some more processes - which should at least get me though the transition to another solution.