Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 325394 - some times the user cannot step for 5 seconds during fast stepping
Summary: some times the user cannot step for 5 seconds during fast stepping
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf (show other bugs)
Version: 7.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 7.0.2   Edit
Assignee: Anton Leherbauer CLA
QA Contact: Pawel Piech CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-15 16:39 EDT by Dobrin Alexiev CLA
Modified: 2010-11-05 09:24 EDT (History)
2 users (show)

See Also:


Attachments
Proposed fix (2.17 KB, patch)
2010-09-17 06:55 EDT, Anton Leherbauer CLA
aleherb+eclipse: iplog-
Details | Diff
Follow-up fix (1.59 KB, patch)
2010-11-05 09:21 EDT, Anton Leherbauer CLA
aleherb+eclipse: iplog-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dobrin Alexiev CLA 2010-09-15 16:39:29 EDT
Build Identifier: Version: 3.6.0 

DsfSourceDisplayAdapter.startDisplayJob() is called with eventTriggered = true.
A DisplayJob() with eventTriggered = true is created and scheduled. 
Right after that DsfSourceDisplayAdapter.startDisplayJob() is called with eventTriggered = false.
The previous DisplayJob is canceled :fRunningDisplayJob.cancel(); but its run() never had chance to execute, therefore never call doneStepping(fResult.getDmc());
That will prevent StepController to allow further stepping. 
 
I did  simple experimenting by adding these two lines of code just before:  fRunningDisplayJob.cancel(); 
                if( fRunningDisplayJob.fEventTriggered) 
                                doneStepping(fRunningDisplayJob.fResult.getDmc());
After adding these two lines the issue is gone!
 
I don’t understand DsfSourceDisplayAdapter 100% yet, but I wanted to check with you if this is correct use case for the class.
If a display job is to be canceled should we make sure that doneStepping is called on its behalf. 
Or may be if the display job is preempted by another display job we somehow transfer the doneStepping responsibility. 
 
Or may be somehow the use case I’m describing should be prevented in another place. 


Reproducible: Always
Comment 1 Dobrin Alexiev CLA 2010-09-15 16:42:18 EDT
I think your analysis makes perfect sense and so does the suggested fix.  I only wonder why the call to displaySource(eventtriggered==false) overrides the one with eventTriggered==true.  Toni added this feature he may be able to comment on it better.

Cheers,
Pawel
Comment 2 Anton Leherbauer CLA 2010-09-17 06:52:43 EDT
(In reply to comment #1)
> I think your analysis makes perfect sense and so does the suggested fix.  I
> only wonder why the call to displaySource(eventtriggered==false) overrides the
> one with eventTriggered==true.  Toni added this feature he may be able to
> comment on it better.

Any source display request coming in after the other has higher priority, that's why any previous request is obsolete and can be canceled.  We just need to make sure that doneStepping is called if necessary.

With DSF-GDB I never get into this situation, except when a breakpoint is hit. In this case we receive two suspended events (one normal suspended event and a breakpoint-hit event which is also a suspended event).  In this particular case it would be actually wrong to perform doneStepping() for the canceled job, because the latter should do it instead.  

I'll attach a patch.
Comment 3 Anton Leherbauer CLA 2010-09-17 06:55:52 EDT
Created attachment 179101 [details]
Proposed fix

Could you try this patch if it works for you? Thanks.
Comment 4 Dobrin Alexiev CLA 2010-09-17 14:09:41 EDT
Thanks you Tony. The fix works for me. 
With it I cannot reproduce the issue anymore. 

Is it possible to have the fix in both 7.0.x and 8.x?
Comment 5 Anton Leherbauer CLA 2010-09-22 09:07:05 EDT
Targeted for 7.0.2.
Comment 6 Dobrin Alexiev CLA 2010-09-23 10:48:27 EDT
Thanks, again.
Comment 7 Anton Leherbauer CLA 2010-09-28 06:30:08 EDT
Fixed in HEAD (8.0) and cdt_7_0 (7.0.2).
Comment 8 Anton Leherbauer CLA 2010-11-05 09:21:51 EDT
Created attachment 182470 [details]
Follow-up fix

We found another case of this issue and this patch addresses it.
Comment 9 Anton Leherbauer CLA 2010-11-05 09:24:41 EDT
Committed the follow-up fix to HEAD and cdt_7_0.