I have some code, a class method on a Ruby class FootballSeries.find(123)
, which performs an API call… owing to concerns about thread safety, only one thread may enter this method at a time. Due to some recent changes on the API, I have also support the following… FootballSeries.find('Premiership')
, the second variety (see implementation below) simply makes an interim call to see if there's an ID can be found, then recursively calling itself using the ID.
class FootballSeries
@find_mutes = Mutex.new
class << self
def find(series_name_or_id)
@find_mutex.synchronize do
if series_name_or_id.is_a?(String)
if doc = search_xml_document(series_name_or_id)
if doc.xpath('//SeriesName').try(:first).try(:content) == series_name_or_id
@find_mutex.unlock
series = find(doc.xpath('//seriesid').first.content.to_i)
@find_mutex.lock
return series
end
end
elsif series_name_or_id.is_a?(Integer)
if doc = xml_document(series_name_or_id)
Series.new(doc)
end
end
end
end
end
end
Without lines 9 and 11, there's a recursive mutex lock: deadlock
error (which makes enough sense… therefore my question is, may I release and re-lock the mutex. (I re-lock, so that when synchronize
exits, I won't get an error unlocking a mutex that I don't own… but I haven't tested if this is required)
开发者_开发知识库Is this a sane implementation, or would I be better served having find()
call two individual methods, each protected with their own mutex? (example find_by_id
, and find_by_name
)
What I have now works (or at least appears to work).
Finally, bonus points for - how would I test such a method for safety?
This doesn't look good to me, as @find_mutex.unlock
will allow another method(s) to enter at the same time. Also, I don't think using recursion is usual for this kind of method dispatch - actually you have two methods stuffed into one. I would certainly separate these two, and if you want to be able to call one method with different argument types, just check the argument's type and invoke one or the other. If you don't need to expose find_by_id
and find_by_name
, you can make them private, and put mutex.synchronize
only in find
.
精彩评论