Bugzilla – Full Text Bug Listing
| Summary: | [patch] Debug view may show duplicate threads | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Stepan Vavra <vavra.stepan> |
| Component: | Debug | Assignee: | JDT-Debug-Inbox <jdt-debug-inbox> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | jarthana, Michael_Rennie, sarika.sinha, vavra.stepan |
| Version: | 4.3 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | stalebug | ||
| Attachments: | |||
With the last sentence, I just wanted to say I don't know whether to modify a master branch or to create a new one based on some tag...) (In reply to Stepan Vavra from comment #1) > With the last sentence, I just wanted to say I don't know whether to modify > a master branch or to create a new one based on some tag...) You need to create the patch on master. I requested a pull on Github. See https://github.com/eclipse/eclipse.jdt.debug/pulls (In reply to Stepan Vavra from comment #3) > I requested a pull on Github. > See https://github.com/eclipse/eclipse.jdt.debug/pulls Thank you for the patch Stepan, It might be better to make the threads collection synchronized rather than start adding in a bunch of sync blocks - the main problem being that the target + threads are queried all over the place (from breakpoints, filters, views, actions, etc) and if we start by simply sync'ing access to the threads we can minimize the risk of deadlock (hopefully). Hi Michael, I totally agree it's not the best solution. Do you think it's better to not apply my patch in a favour of no risk of deadlocks when working with the thread list? I actually really need to fix this for my implementation of JDWP agent for JavaPathfinder (https://bitbucket.org/stepanv/jpf-jdwp). I can propose a solution of some concurrent thread manager for the JDI model because I need to fix it so badly, although I'm afraid if I can propose something that matches your idea of a good solution or a good code design in general... Thanks, Stepan (In reply to Stepan Vavra from comment #5) > Hi Michael, > I totally agree it's not the best solution. > Do you think it's better to not apply my patch in a favour of no risk of > deadlocks when working with the thread list? > Perhaps if we start by changing the collection itself to be a SynchronizedCollection and then use sync blocks where we iterate would be a good place to start. That way we could get rid of getThreadIterator method completely (and all of the coping it does). I'm assuming this is not a priority for you. Therefore, I'll take a look at it. By the way, what about creating a dedicated Thread Manager class that guarantees an integrity for any method it exposes? Anyway, I'll see what I can do about it. Thanks, Stepan I cannot assign this bug to me, can I? (In reply to Stepan Vavra from comment #7) > I'm assuming this is not a priority for you. Therefore, I'll take a look at > it. > Any time we have someone willing to put time into fixing a bug it becomes a priority. > By the way, what about creating a dedicated Thread Manager class that > guarantees an integrity for any method it exposes? > To be honest I would prefer to stay away from yet another manager - we have too many of those in Debug as it is now :) (In reply to Michael Rennie from comment #9) > (In reply to Stepan Vavra from comment #7) > > I'm assuming this is not a priority for you. Therefore, I'll take a look at > > it. > > > > Any time we have someone willing to put time into fixing a bug it becomes a > priority. > > > By the way, what about creating a dedicated Thread Manager class that > > guarantees an integrity for any method it exposes? > > > > To be honest I would prefer to stay away from yet another manager - we have > too many of those in Debug as it is now :) I just saw that you posted a pull request a while back for this. Unfortunately I don't monitor the github clone. Can you provide the fix as a Gerrit contirbution? http://wiki.eclipse.org/Gerrit http://wiki.eclipse.org/Development_Resources/Contributing_via_Git @Stephan Do you want to continue to work on this? Moving it out of 4.7 as no one is currently planning to work on this. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |
Created attachment 236144 [details] (Patch for commit 43b0dc87e902de5a9bfbb0b494e57132fd990253). This is not the best solution (as the TODOs in this file are pointing out - there is a need to create a thread safe thread list) When debugging a Java application, in some circumstances, the Debug view may show duplicate threads. For instance, multiple "main" threads may be shown. Cause: There is a race condition in the thread start event handler (JDIDebugTarget$ThreadStartHandler.handleEvent()) - the implementation doesn't guarantee that a certain thread is added just once to the thread list. Fix suggestion: The logic behind the call of methods "findThread" and "createThread" should be atomic/synchronized somehow (see attached patch file). Is it possible to backport this fix to any current Eclipse IDE release (For example Kepler). I don't know whether it's possible or how exactly it works when it comes to fixing releases. If it makes it easier I can request for a pull on Github. I just don't know whether to Thanks, Stepan