Deadlock problem when using Split.stop(String) with more than one subSimon name and SimonManager.clear()

[coding4z]

I opened the problem here and in google code just in case....

What steps will reproduce the problem?

1. Started 10 threads, each thread loops the following: a) Split s = SimonManager.getStopwatch("TEMP").start(); b) s.stop("" + r.nextInt(2)); //where r is java.util.Random to produce a subSimon of name "0" or "1"
2. Started 1 thread that loops the following: a) System.out.println(org.javasimon.utils.SimonUtils.simonTreeString(SimonManager.getRootSimon())); b) SimonManager.clear(); c) Thread.sleep(1000);
3. The threads started in step #1 end up deadlocking.

What is the expected output? What do you see instead? Expect this application to run without deadlocking.

What version of the product are you using? On what operating system? javasimon-core-4.0.1 I have run this on both MAC OS X and z/OS both result in the same problem.

Please provide any additional information below. Two observations: 1) When Split.stop(String subSimon) only uses a single subSimon name the deadlock doesn't appear to occur, only when two or more subSimon names are used. 2) When SimonManager.clear() is not issued the deadlock doesn't appear to occur

I have provided two files: 1) My test cases that can recreate the problem. https://code.google.com/p/javasimon/issues/attachmentText?id=130&aid=1300000000&name=SimonThreadTest.java&token=ABZ6GAeoOzda3g7AfVZ5xlNyDUMwLkswqQ%3A1419892900998 2) A thread dump showing the blocked threads and the stack trace of where they are blocked. https://code.google.com/p/javasimon/issues/attachmentText?id=130&aid=1300000001&name=thread+dump.txt&token=ABZ6GAdYWGNdlPijB8WNpGTszOa9AI_7FA%3A1419892900998

[coding4z]

My first observation is incorrect, I was able to recreate the problem with a single subSimon as well, but so far I cannot recreate the problem when using Split.stop() only Split.stop(String subSimon).

incorrect statement: 1) When Split.stop(String subSimon) only uses a single subSimon name the deadlock doesn't appear to occur, only when two or more subSimon names are used.

[coding4z]

did not mean to click "close"

[virgo47]

Tricky one, seems to be easy to fix, but there are things I don't understand. All threads share the same TEMP stopwatch and call stop(randomNumber) on it. This method eventually synchronizes on the TEMP stopwatch and within that lock tries to create new sub-stopwatch using another lock for EnabledManager and then another one on Stopwatch instance again to update it's children list.

Easy fix was to do the stuff on behalf sub-stopwatch out of the synchronized block on the main stopwatch (TEMP). I can't reproduce any dead-lock then. Originally I was puzzled why:

• the thread dump shows that the first above Split.stop is different for the two interlocked threads (reported in deadlock section)
• the two TestThreads seem to be interlocked and should be enough for dead-lock, but disabling that thread clearing and printing tree would not show any deadlock

Then these two things clicked - that clear is what caused the deadlock - because:

• TEMP (call it X) before clear was taken in thread 1
• manager was cleared
• another TEMP (call it Y) was created in thread 2 and obtained, stop is called and synchronized section is entered
• thread 1 continued and decided to create TEMP.child (any will do now) - this process MUST inform manager about new simon and does not use original X object at all - just its name that is the same like Y instance (TEMP), hence lock on manager is obtained
• thread 1 further continues and creates TEMP.child and wants to update TEMP's list of children - this was also synchronized - and this synchronization was superfluous as the list itself is synchronized object (CopyOnWriteArrayList) - but this lock is taken by thread 2 already (here the new instance of TEMP is used, Y, because manager does not know about the X anymore)
• thread 2 continues and when it needs to get sub-simon it needs manager's lock - already taken by thread 1 - game over

Fix consists of:

• unnesting the synchronized sections in stop(subsimon) itself - manager is locked after "this" is released
• removed superfluous synchronized on org.javasimon.AbstractSimon#addChild