Bug 221224 - debugger mi target locking problems
Summary: debugger mi target locking problems
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug (show other bugs)
Version: 5.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 5.0 M6   Edit
Assignee: Anton Leherbauer CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-03-03 16:01 EST by Elena Laskavaia CLA
Modified: 2008-06-22 03:44 EDT (History)
0 users

See Also:


Attachments
5.0 ppatch (29.65 KB, patch)
2008-03-03 16:01 EST, Elena Laskavaia CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Elena Laskavaia CLA 2008-03-03 16:01:01 EST
Created attachment 91439 [details]
5.0 ppatch

Several problems were reported about locking issues using Target#Lock.aquire.
In essence some debug command lock the target using custom made lock in Target class in mi plugin and do not release it properly or release it twice.
In first case ui usually dies, and in second future debugging is usually paralyzed
until IDE re-start. 

I tried to fix it using pr 215553, but either I miss some or later patches re-create the problem again. 

To properly fix the problem we need to get rid of these custom locks and use java synchronize locks which are error-free for missed unlock and double unlocks. Other than that these two methods should work the same (both Targt#Lock and synchronize locks are thread re-entrant)

It is pretty big patch because this spread around the code, but all changes are pretty much the same. Code can also use some formatting after the patch because I tried not to reformat too much to minimize the size of it.
Comment 1 Anton Leherbauer CLA 2008-03-12 11:42:22 EDT
Patch looks good to me. It is a breaking change for clients of class 'Target', but the fix is straightforward.
Comment 2 Elena Laskavaia CLA 2008-03-12 13:00:56 EDT
In unlikely case when somebody used mi Target locking directly
we should probably leave a comment for getLock like

/**
  Return lock object for target. Replacement for lockTarget and releaseTarget methods. Use as synchronization object:
  new code:
  synchronized (target.getLock()) {
     ...
  }
  old code: 
  target.lockTarget();
  try {
    ...
  } finally {
    target.releaseTarget();
  }
  
*/
Comment 3 Anton Leherbauer CLA 2008-03-13 05:17:10 EDT
(In reply to comment #2)
> In unlikely case when somebody used mi Target locking directly
> we should probably leave a comment for getLock like
> /**
>   Return lock object for target. Replacement for lockTarget and releaseTarget
> methods. Use as synchronization object:
>   new code:
>   synchronized (target.getLock()) {
>      ...
>   }
>   old code: 
>   target.lockTarget();
>   try {
>     ...
>   } finally {
>     target.releaseTarget();
>   }
> */

Thanks, I have applied the patch including the comment.