rachavz / reflections

Automatically exported from code.google.com/p/reflections
Do What The F*ck You Want To Public License
0 stars 0 forks source link

Store still not threadsafe. #92

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
While using the parallel executor I found that a certain unit test sporadically 
failed. Upon further inspection I found that Reflections did not produce 
consistent log messages:

    Reflections took 67 ms to scan 1 urls, producing 1 keys and 4 values [using 4 cores]
    Reflections took 67 ms to scan 1 urls, producing 2 keys and 4 values [using 4 cores]
    Reflections took 71 ms to scan 1 urls, producing 1 keys and 3 values [using 4 cores]

I saw Issue 68 and had a look. The problem seems to be that the Store class is 
still not threadsafe. It is not enough to ensure that the inner MultiMap 
supports concurrent r/w access: this must also hold for the outer map. 
Specifically, the outer map must implement ConcurrentMap and any insertion must 
happen through #putIfAbsent.

I made some changes to the Store class along those lines and indeed the problem 
went away. This is no proof, however. Right now I do not have time to further 
look into this issue (for the time being I simply disabled parallel execution), 
but if needed I can assist in resolving this bug in the near future.

Original issue reported on code.google.com by stephan...@gmail.com on 16 Nov 2011 at 10:58

GoogleCodeExporter commented 9 years ago
fixed on 0.9.7-RC1 and trunk. comments?

Original comment by ronm...@gmail.com on 2 May 2012 at 3:54

GoogleCodeExporter commented 9 years ago
I'm sorry, this is not fixed. I just re-enabled parallel execution and ran some 
tests: the number of reported values varies still.

I noticed that r151 introduces the use of an AtomicInteger in Reflections.java, 
but that seems to be an attempt to solve the problem at the wrong level (or is 
that change in the context of some other ticket?) The Store class appears 
unmodified. 

Original comment by stephan...@gmail.com on 2 May 2012 at 4:23

GoogleCodeExporter commented 9 years ago
patch?

Original comment by ronm...@gmail.com on 2 May 2012 at 5:03

GoogleCodeExporter commented 9 years ago
fixed on trunk

Original comment by ronm...@gmail.com on 9 May 2012 at 12:43

GoogleCodeExporter commented 9 years ago
fixed 0.9.7

Original comment by ronm...@gmail.com on 22 May 2012 at 7:47

GoogleCodeExporter commented 9 years ago

Original comment by ronm...@gmail.com on 22 May 2012 at 7:58

GoogleCodeExporter commented 9 years ago
yet again, fixed on trunk (0.9.9-SNAPSHOT). parallel scanning should be much 
better now and the store is concurrent. comments on that? anyone?!?!

Original comment by ronm...@gmail.com on 20 Feb 2013 at 9:18

GoogleCodeExporter commented 9 years ago

Original comment by ronm...@gmail.com on 22 Nov 2013 at 8:51