digital-fabric/affect

Thread-safety?

timhabermaas opened this issue · 2 comments

Affect.capture doesn't seem thread-safe because it mutates the current context (@@current) class variable and Affect is shared across threads.

For example:

t1 = Thread.new  {
  Affect.capture(log: -> (message) { puts "logging 1: #{message}" }) do
    Affect.perform(:log, "foo")
  end
}

t2 = Thread.new {
  Affect.capture(log: -> (message) { puts "logging 2: #{message}" }) do
    Affect.perform(:log, "bar")
  end
}
t1.join
t2.join

might end up printing "logging 2: foo\nlogging 2: bar\n" instead of "logging 1: foo\nlogging 2: bar\n".

This specific issue can be fixed by using Thread.current[:current_ctx] to attach the current context to the current thread (do you want me to open a PR?). However: I think this will lead to other issues when using Threads in user-land [1].

So I looked into using the fiber implementation which doesn't mutate global state, so the above code works fine. I believe it has the same problem when using Thread.new around perform as the Thread.current solution, though. Fibers don't seem to play nicely with threads.

Do you have any experience using your affect library in a multi-threaded environment? Maybe it's best to simply ban threads from user-land?

[1] The part of a code base which only cares about perform, not about how to handle the effects.

For my own use, up until now I have used a single global context, so there was no problem with multi-threaded code.

For the moment, the behaviour of concurrent capture blocks is undefined. Using a thread-local variable is a solution, but I think a more comprehensive solution would be to allow threads to "inherit" the context of the thread doing the call to Thread.new. This would necessitate a patch to Thread.new. Also note that in Ruby thread-local variables are actually fiber-local, so this might affect the behaviour when running multiple fibers.

If you have a suggestion for how to solve this I'd like to know. Thanks for your interest!

Closing due to inactivity.