Summary: | Local shell reports isActive wrong | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Tools] Target Management | Reporter: | Lothar Werzinger <lothar> | ||||||
Component: | RSE | Assignee: | Martin Oberhuber <mober.at+eclipse> | ||||||
Status: | RESOLVED FIXED | QA Contact: | Martin Oberhuber <mober.at+eclipse> | ||||||
Severity: | minor | ||||||||
Priority: | P3 | Keywords: | helpwanted | ||||||
Version: | 1.0 | ||||||||
Target Milestone: | 2.0.1 | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Whiteboard: | |||||||||
Attachments: |
|
Description
Lothar Werzinger
2006-10-21 11:37:24 EDT
Created attachment 52461 [details]
The modified HostShellAdapter.java from RSE 1.0RC2 that prints the isActive status.
The modified HostShellAdapter.java from RSE 1.0RC2 that prints the isActive status.
Decreasing severity, since the use-case (cdt remote debug) is not a must-have on local (can use cdt local debug there). It is not clear what Platform you observed the problem on. Please always set the Platform to the one where you observed the problem. This will allow us to reproduce and fix the issue on a platform more similar to yours. Use "All" only if you actually reproduced the problem at least on 2 platforms. Ideally, add a platform description template to your bug report, like the one available from http://wiki.eclipse.org/index.php/RSE_1.0_Test_Instructions#Step_4:_Prepare_for_Bug_Reports This is actually important for my application, where I want to use RSE to launch either local or remote. I would rather not work around it if I don't have to. If i's not too hard to fix I'd love to have it fixed before 1.0. Hm, we are really busy fixing other hi-priority issues right now. Would you want to try a fix yourself? Getting a debug version of RSE is really simple, just take the Core "rse-anonymous.psf" CVS project set from http://www.eclipse.org/dsdp/tm/development/cvs_setup.php and in PDE use Import > Team > Project Set. Then, create a Debug Launch Config of "Eclipse Appliation" type. We'll be happy to receive your patch. Created attachment 52645 [details]
patch to enable termination
This patch does not fix the problem with isActive(), but it at least enables to terminate a local shell (clicking on the red square works now). I am still figuring out how the local shell works. It's quite different from the SSH shell. What's needed is that when the ouputstream gets disconnected we have to set isActive to false; We know that the stream goes bad, as the OutputStreamMonitor receives an IOException.
Thanks a lot for this patch! We'll need to go through a few legal questions before I can accept and commit the patch, as outlined on http://www.eclipse.org/dsdp/tm/development/committer_howto.php#external_contrib I'm contacting you directly for that. Legal Message: I, Lothar Werzinger, declare that I developed attached code from scratch, without referencing any 3rd party materials except material licensed under the EPL. I am authorized by my employer to make this contribution under the EPL. Patch applied. Welcome as contributor, Lothar! Your name is now listed on http://www.eclipse.org/dsdp/tm/development/contributors.php and http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.tm.rse/plugins/org.eclipse.rse.services.local/src/org/eclipse/rse/internal/services/local/shells/LocalShellThread.java?rev=HEAD&cvsroot=DSDP_Project&content-type=text/vnd.viewcvs-markup and http://www.eclipse.org/dsdp/tm/development/tm-log.csv The bug remains open until isActive is reported correctly. Shooting for RC1. Problem fixed and committed: [161838] local shell reports isActive() wrong LocalHostShell.java LocalShellThread.java The problem was that the LocalShellThread does not detect the shell process status, and the EOF on the LocalShellOutputReader was never checked. My fix is to check the LocalShellOutputReader.isFinished() in order to see whether the shell is still active; losing the output stream is a safer and easier check than testing the actual process status. When the output stream is lost, the shell thread is terminated as well in order to free up all resources. |