Yes, this modification was only written for 1.0 branch, because I had
the impression that Bill Dortch would do something on the trunk anyway.
However, it is easy to apply to trunk as well, I suspect. I will have a
look tomorrow.
This modification merely solves (as solvable as I possible can do
without refactoring) concurrent access to the method hash map.
I do not have much knowledge on the domain models you use, so I focused
entirely on the specified map.
I followed the source code to find all places in which the map was
exposed directly or indirectly, and analyzed it for race conditions.
I then ran the test cases with a code coverage tool to evaluate the
usage pattern, before I decided on the solution.
During analysis I did found some bad smells..
a) internal collections are made publicly available (without any
description or restrictions in javadoc about semantics)
b) The collection is iterated at caller, when behaviour should be at
callee. (Class-Responsibility)
So even if I supplied a fix, there are still refactoring which could be
done. However, I would rather wait a while until I understand the domain
model better.
Is there any documentation on the jruby architecture? For the core, at
least?
Regards,
/Niels
Charles Oliver Nutter (JIRA) skrev:
[ http://jira.codehaus.org/browse/JRUBY-1259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_112225 ]
Charles Oliver Nutter commented on JRUBY-1259:
----------------------------------------------
So is this only a modification to 1.0 branch? I'd actually like to see a better
method hash on trunk as well, since MethodCache is being voted out and the
original global method cache is just a hash too.
Method access is not threadsafe
-------------------------------
Key: JRUBY-1259
URL: http://jira.codehaus.org/browse/JRUBY-1259
Project: JRuby
Issue Type: Bug
Components: Core Classes/Modules
Reporter: Bill Dortch
Priority: Critical
Attachments: patch-max.txt
Method access (add/remove/search) in JRuby is currently not threadsafe.
I've just made a couple of changes to MethodCache that should help, though I'm
still not convinced that it is safe to use without synchronization. (My changes
are intended to address atomicity issues, but as reported elsewhere, JIT
behavior in sequencing operations may lead to problems if synchronization is
not done properly, or, in this case, at all.)
There are also a number of places that the _methods_ HashMap in RubyModule is
accessed unsafely. This is easy enough to demonstrate:
{code}
class C; end
t1 = Thread.new do
loop do
class C
def a; end
def b; end
def c; end
def d; end
end
end
end
t2 = Thread.new do
loop do
C.send :remove_method, :a rescue nil
C.send :remove_method, :b rescue nil
C.send :remove_method, :c rescue nil
C.send :remove_method, :d rescue nil
end
end
loop do
C.instance_methods
C.instance_methods
C.instance_methods
C.instance_methods
end
{code}
Wait for a moment, and you'll get:
{noformat}
NativeException: java.util.ConcurrentModificationException: null
from HashMap.java:793:in `java.util.HashMap$HashIterator.nextEntry'
from HashMap.java:833:in `java.util.HashMap$EntryIterator.next'
from HashMap.java:831:in `java.util.HashMap$EntryIterator.next'
from RubyModule.java:1597:in `org.jruby.RubyModule.instance_methods'
from RubyModule.java:1624:in `org.jruby.RubyModule.instance_methods'
etc...
{noformat}
This is a fairly benign outcome, a stack trace spewn across one's screen being
a pretty clear indication that something has gone amiss. But there are other
access points where more subtle, possibly undetected (or misinterpreted)
problems might arise. I suspect, for example, that JRUBY-1251 is one of these
(though it may be resolved by my MethodCache fix.)
It would be easy enough just to add synchronization where it's currently
missing, but I'm working on a MethodMap class (part of my refactoring
experiment in the bdortch/bnw branch) that will provide a more performant
solution. I'll try to get that piece into trunk sooner rather than later.
-Bill
---------------------------------------------------------------------
To unsubscribe from this list please visit:
http://xircles.codehaus.org/manage_email
