Firstly, before you all say you should never use Thread#kill and link to Headius post, this is not the same issue which he explains, but it is somewhat related.
Secondly, before you ask why are you using Thread#kill anyway, or why should we worry about anyone who is. There is one scenario where Threads being #killed is quite likely and there are probably a bunch of people who run into it without knowing. When a ruby process terminates all Threads other than the main one are #killed (the main Thread gets an exception raised). So if you had a ruby process with a bunch of threads and this process received a TERM signal, all those Threads are going to get #killed unless you specifically handle the TERM signal and instruct/wait for all threads to finish. That is of course what you should do but I am guessing there are plenty of people who don’t.
ActiveRecord transactions and Thread#kill
First let me say your ActiveRecord transactions could already be partially committed if you use Thread#kill or Thread#raise due to the reasons Headius’ explains. But the chance of this happening due to those reasons is lower than due to this new one. Here’s the ActiveRecord transaction code. I’ve posted the Rails 2.2 version because it’s much simpler and less code than the current 3.0 source. But 3.0 is still vulnerable to the same issue:
def transaction(start_db_transaction = true) transaction_open = false begin if block_given? if start_db_transaction begin_db_transaction transaction_open = true end yield end rescue Exception => database_transaction_rollback if transaction_open transaction_open = false rollback_db_transaction end raise unless database_transaction_rollback.is_a? ActiveRecord::Rollback end ensure if transaction_open begin commit_db_transaction rescue Exception => database_transaction_rollback rollback_db_transaction raise end end end
Now imagine you’re using it like this
ActiveRecord::Base.transaction do user_1.update_attribute :something, 1 ... user_n.update_attribute :something, n end
If Thread#kill is called anywhere inside of that block the rest of the updates will not occur but the transaction will be committed.
How does it happen?
The reason this happens and why it’s a little unexpected is because Thread#kill will bypass all code _including_ rescue blocks and jump immediately to ensure blocks. Many people code with the assumption if an ensure block is hit without a rescue block being hit the code must have finished executing. And this is the assumption made in the ActiveRecord transaction code. Imagine we move from the middle of the block passed to #transaction (where Thread#kill was called) down to the start of the ensure block. transaction_open will evaluate to true (which it would not have were an exception raised since the rescue block sets transaction_open to false) and commit_db_transaction will be called.
In contrast, for this to occur due to the Headius’ issue first an exception would have to be raised inside the transaction block and _then_ Thread#raise or Thread#kill would have to be called inside the rescue block before transaction_open is set to false, a much smaller area of attack.
Can this be fixed?
Yes and no. Setting a local variable to true after the yield statement is a simple way to determine if the block passed to yield ran to completion:
begin yield completed = true ensure puts "entire block ran!" if completed end
This assumes the code in the block doesn’t use certain flow control statements. Those statements are return, break, next, retry, redo and throw. If one of these statements is used to break out of the block prematurely it will effectively act just like Thread.kill. The interpreter will immediately jump to the ensure block bypassing everything else including the local variable being set. Which means if you relied on the local variable being set to commit the transaction then any transaction block with one of those statements would always be rolled back.
In MRI 1.8 and Rubinius Thread.current.status will be set to ‘aborting’ if the current thread is in the state of being killed. So we can detect if we are in the ensure block because of a Thread#kill rather than one of the flow control statements.
begin yield ensure puts "enitre block ran!" if Thread.current.status != 'aborting' end
The problem is this doesn’t work at all in 1.9 and isn’t consistent in JRuby. If all interpreters behaved as 1.8 does then fixing this issue would be trivial and have no downsides. Knowing whether you are in an ensure block prematurely because the programmer intended so via flow control statements versus due to an aborting thread seems fairly important. So I believe that 1.9 and JRuby should copy this behavior or at least provide some other similar mechanism.
Here’s a patch which fixes the issue for MRI 1.8 and Rubinius:
Thanks to Marcos Toledo for brainstorming the issue with me and helping me proofread