Community
Participate
Working Groups
Build Identifier: Sequence.executeStep uses a RequestMonitor instance whose done method is called when the step is done. The implementation of RequestMonitor.done() calls handleCompleted via executor. However, sometimes the executor may be gone. In that case, RejectedExecutionException is raised, and RequestMonitor.handleRejectedExecution is called. That method calls done on the parent monitor. For Sequence, the parent monitor is RequestMonitorWithProgress whose 'handleCompleted' method does not do much. As result, the 'finish' method of Sequence is never called, and any code blocked in Sequence.get remains blocked forever. Attached patch fixes this in most straightforward way. It might be better to notify about the error, but it's not obvious to me how. Reproducible: Sometimes Steps to Reproduce: This reproduces in our product that implements run via start-debug-session-detach-terminate sequence.
Created attachment 179365 [details] Initial patch
List discussion at http://dev.eclipse.org/mhonarc/lists/cdt-dev/msg20416.html
> How do you suggest to handle errors? IIUC, for the case of final launch > sequence, Sequence.fRequestMonitor is not actually used by the clients, > so even if we call setStatus on it, nobody will notice. I checked in a fix that causes the Sequence.get() call to throw an exception. This exception will need to be handled in the launch delegate. Please let me know if this fix works for you. Mikhail, please review when you get a chance.
(In reply to comment #3) > I checked in a fix that causes the Sequence.get() call to throw an exception. > This exception will need to be handled in the launch delegate. Pawel, you no longer attach patches? :-)
*** cdt cvs genie on behalf of ppiech *** Bug 325943 - [concurrent] Robustify Sequence against RejectedExecutionException [*] DsfSequenceTests.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.tests.dsf/src/org/eclipse/cdt/tests/dsf/concurrent/DsfSequenceTests.java?root=Tools_Project&r1=1.3&r2=1.4 [*] Sequence.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/Sequence.java?root=Tools_Project&r1=1.6&r2=1.7
Looks good to me.
(In reply to comment #4) > Pawel, you no longer attach patches? :-) I thought that with CDT genie we no longer need to, right?
(In reply to comment #7) > (In reply to comment #4) > > Pawel, you no longer attach patches? :-) > I thought that with CDT genie we no longer need to, right? That was a gray area for me. The CDT Genie sometimes takes a while to kick in (I've seen some instances of it taking weeks, although maybe that was at the beginning). Also, one cannot revert a patch unless we have it attached. And we cannot apply it another branch, if needed. The bugzilla patch diff is also nice to use, when a patch is attached. As you can see, I really like it when patches are attached. But that is just my own preference.
FWIW, I agree with Marc. The auto-added changes have a few problems: 0. CDT uses CVS 1. There's one link per file, which is inconvenient for patches that touch more than one file. 2. Getting individual patch requires quite some clicks. 3. Each patch only has basename of the file in the header, which makes applying hard. So, it would be nice that either CDT switch to a version control system that has atomic commits, or that patches be attached to bugzilla.
