From 32b75bf62c0c1770b68e7e1a9918718943d1c04c Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 3 Aug 2015 21:40:51 +0200 Subject: [PATCH] Reset the owner ivar in LRU#synchronize Without this the following could happen: 1. Thread A acquires the lock and sets the ownership to A. 2. Thread A yields and returns 3. Thread B tries to acquire the lock 4. At this exact moment Thread A calls the "synchronize" method again and sees that the "owner" variable is still set to Thread A 5. Both thread A and B can now access the underlying data in parallel, possibly leading to corrupted objects This can be demonstrated using the following script: require 'oga' lru = Oga::LRU.new(64) threads = 50.times.map do Thread.new do loop do number = rand(100) lru[number] = number end end end threads.each(&:join) Run this for a while on either JRuby or Rubinius and you'll end up with errors such as "ConcurrencyError: Detected invalid array contents due to unsynchronized modifications with concurrent users" on JRuby or "ArgumentError: negative array size" on Rubinius. Resetting the owner variable ensures the above can never happen. Thanks to @chrisseaton for bringing this up earlier today. --- lib/oga/lru.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/oga/lru.rb b/lib/oga/lru.rb index a3d210a..be330e3 100644 --- a/lib/oga/lru.rb +++ b/lib/oga/lru.rb @@ -137,8 +137,10 @@ module Oga if @owner != Thread.current @mutex.synchronize do @owner = Thread.current + retval = yield + @owner = nil - yield + retval end else yield