karouani / javasimon

Automatically exported from code.google.com/p/javasimon
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

EnabledManager.getSimon is not threadsafe if Simon is altered by Callback onSimonCreate #129

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a Callback that adds an object to a Stopwatch's attributes map from 
the onSimonCreated method -- ideally this Simon is unavailable to any threads 
until the callbacks are complete.

2. Create a test case where you have say 5 threads that start concurrently, all 
calling getStopwatch and checking that that attribute object exists when the 
getStopwatch call returns...

3. Because of the lack of synchronization between the SimonManager's 
createOrReplaceUnknownSimon (sync'd on the manager lock itself) and the call to 
allSimons.get (sync'd on internal lock of ConcurrentHashMap) in 
getOrCreateSimon you have a case where the first thread in can get up to the 
get/put but -is in the process of calling the callbacks- and another thread 
uses the get and receives a non-null Simon so it does NOT wait for the 
callbacks to complete.

What is the expected output? What do you see instead? I would expect the 
manager to guarantee that before a Simon is exposed to any thread (i.e. before 
getStopwatch returns), all callbacks onSimonCreated have completed. All waiting 
threads should only get notified after the callbacks are done their work.

What version of the product are you using? 3.5.1
On what operating system? Windows 7 Pro

Please provide any additional information below.

Original issue reported on code.google.com by ssmcguard@gmail.com on 3 Oct 2014 at 10:00

GoogleCodeExporter commented 8 years ago
Thank you for the report. Originally this was synchronized on the manager 
indeed (long time ago). Generally callbacks on simons are out of locked 
section, because they can take longer to do their stuff - often unrelated to 
the simon internal data (e.g. logging).

However initialization of various attributes in onSimonCreated should be safe, 
you're right about that. I moved synchronized from createOrReplaceUnknownSimon 
to getOrCreateSimon. That should do the trick.

Will be released as 3.5.2 hopefully soon and later in 4.0.0 too.

Original comment by virgo47 on 4 Oct 2014 at 7:20

GoogleCodeExporter commented 8 years ago

Original comment by virgo47 on 5 Oct 2014 at 4:10

GoogleCodeExporter commented 8 years ago
Java Simon 3.5.2 was released to Maven Central.

Original comment by virgo47 on 14 Oct 2014 at 7:58