Bug 327025

Comment 1 Pawel Piech 2010-10-05 13:09:51 EDT

Created attachment 180259 [details]
Fix

This fix takes care of the problem as reproduced by the suggested method.  BTW, I first tried to reproduce the problem by clicking fast on the breakpoint group (to enable/disable breakpoint), until I realized that double-clicking a tree item expanded/collapsed it too.

The fix is to return an error from the cache when the data has been reset by another event.  Before the change, the cache would return a child count '0' for the root node causing it to collapse.  However, this fix is sort of a band-aid.  The breakpoint VM implementation was something of an experiment (as was the platform one) and it's likely that we'll find more issues as we use it more.  So it would be good to proactively invest time in tests and architecture changes.

Comment 2 Pawel Piech 2010-10-05 13:11:36 EDT

I committed the fix.  Patrick please mark bug as verified if it solves the problem for you.

Comment 3 Patrick Chuong 2010-10-05 13:31:56 EDT

I still see the tree collapsed problem. Using gdb you can simulate what I see in our dsf base debugger, fire two change events in the BreakpointVMProvider for each platform change event. By adding a loop in the BreakpointVMProvider.IBreakpoitnListener.breakpointsChange handler to call handleEventInExecThread twice.

Also, I get a nullpointer in the second update in the patch, where getData() can be null, with or without the change in BreakpointVMProvider.

Comment 4 Pawel Piech 2010-10-05 13:38:42 EDT

(In reply to comment #3)
> I still see the tree collapsed problem. Using gdb you can simulate what I see
> in our dsf base debugger, fire two change events in the BreakpointVMProvider
> for each platform change event. By adding a loop in the
> BreakpointVMProvider.IBreakpoitnListener.breakpointsChange handler to call
> handleEventInExecThread twice.
That's what I did also.  Maybe you can post a code snippet of your loop.

> 
> Also, I get a nullpointer in the second update in the patch, where getData()
> can be null, with or without the change in BreakpointVMProvider.
Don't see this either.  Can you give the trace?

Comment 5 Patrick Chuong 2010-10-05 13:57:54 EDT

here is the change that I put in to simulate multipel change events

public void breakpointsChanged(...) {
    for (int i = 0; i < 2; ++i)
        handleEventInExecThread(new BreakpointsChangedEvent 
            (BreakpointsChangedEvent.Type.CHANGED, breakpoints));
}

I am not sure what trace you are looking for. Can you let me know which trace option you want me to enable?

here is the nullpointer exception:

Caused by: java.lang.NullPointerException
	at org.eclipse.cdt.dsf.debug.ui.viewmodel.breakpoints.BreakpointOrganizerVMNode$3.handleCompleted(BreakpointOrganizerVMNode.java:84)
	at org.eclipse.cdt.dsf.concurrent.RequestMonitor$2.run(RequestMonitor.java:291)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:134)
	... 23 more

here is the delta trace:

RECEIVED DELTA: Model Delta Start
	Element: org.eclipse.cdt.dsf.debug.ui.viewmodel.breakpoints.BreakpointVMInput@b5723eb3
		Flags: CONTENT | 
		Index: 0 Child Count: -1
Model Delta End

STATE APPEND BEGIN: null
	APPEND DELTA: Model Delta Start
  Element: org.eclipse.cdt.dsf.debug.ui.viewmodel.breakpoints.BreakpointVMInput@b5723eb3
      Flags: CONTENT | EXPAND | 
      Index: -1 Child Count: 1
    Element: L/java/src/main.java
        Flags: CONTENT | EXPAND | SELECT | 
        Index: 0 Child Count: 1
      Element: Java Line Breakpoints
          Flags: CONTENT | EXPAND | 
          Index: 0 Child Count: 1
Model Delta End

	APPEND OUTSTANDING RESTORE: Model Delta Start
  Element: org.eclipse.cdt.dsf.debug.ui.viewmodel.breakpoints.BreakpointVMInput@b5723eb3
      Flags: CONTENT | EXPAND | 
      Index: -1 Child Count: 1
    Element: L/java/src/main.java
        Flags: CONTENT | EXPAND | SELECT | 
        Index: 0 Child Count: 1
      Element: Java Line Breakpoints
          Flags: CONTENT | EXPAND | 
          Index: 0 Child Count: 1
Model Delta End

	SKIPPED: L/java/src/main.java
	SKIPPED: Java Line Breakpoints
STATE APPEND COMPLETE Model Delta Start
  Element: org.eclipse.cdt.dsf.debug.ui.viewmodel.breakpoints.BreakpointVMInput@b5723eb3
      Flags: CONTENT | EXPAND | 
      Index: -1 Child Count: 1
    Element: L/java/src/main.java
        Flags: CONTENT | EXPAND | SELECT | 
        Index: 0 Child Count: 1
      Element: Java Line Breakpoints
          Flags: CONTENT | EXPAND | 
          Index: 0 Child Count: 1
Model Delta End


I workaround the nullpointer issue by checking data is null or not, if null than set child count to zero.

Comment 6 Pawel Piech 2010-10-05 14:10:19 EDT

Did you actually apply the patch or just make the corresponding changes?  Because, I changed handleCompleted() to handleSuccess().

Comment 7 Patrick Chuong 2010-10-05 14:27:06 EDT

I didn't apply the patch, after changing the handler to succeed, it works very nice. Thank you for the quick fix. The fix is pretty minor, can it be port back to CTD 7.x?

I don't see any where I can mark this bug verified, maybe I am not a committer?

Comment 8 Pawel Piech 2010-10-05 14:29:55 EDT

You can just change the status from Resolved to Verfied :-)

Comment 9 Patrick Chuong 2010-10-05 14:37:54 EDT

I don't think I could. The status is a link, under the 'Additional Comments' edit control and at the top. I can click on it, but doesn't allow me to change the state of this Bug. Am I looking at the right place?

Comment 10 Pawel Piech 2010-10-05 14:55:43 EDT

Yes, I guess it is a committer thing, which I think you ought to be by now...