Bug 161838 - Local shell reports isActive wrong
Summary: Local shell reports isActive wrong
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 1.0   Edit
Hardware: All All
: P3 minor (vote)
Target Milestone: 2.0.1   Edit
Assignee: Martin Oberhuber CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2006-10-21 11:37 EDT by Lothar Werzinger CLA
Modified: 2011-05-25 10:09 EDT (History)
0 users

See Also:


Attachments
The modified HostShellAdapter.java from RSE 1.0RC2 that prints the isActive status. (3.61 KB, text/plain)
2006-10-21 11:38 EDT, Lothar Werzinger CLA
mober.at+eclipse: iplog+
Details
patch to enable termination (742 bytes, patch)
2006-10-25 01:36 EDT, Lothar Werzinger CLA
mober.at+eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lothar Werzinger CLA 2006-10-21 11:37:24 EDT
I observed this problem with RSE 1.0RC1 and RSE 1.0RC2

The remote shell(s) reports isActive correctly, Local does it wrong.

This affects correct termination of a process that uses the Local shell like https://bugs.eclipse.org/bugs/show_bug.cgi?id=158786.

I changed the HostShellAdapter so that I get prints in the destroy() and
exitValue(). See next attachment.

remote shell (Windows dstore):
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=false

remote shell (Linux SSH):
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=false

local shell:
exitValue() - hostShell.isActive()=true
## here I have to manually terminate the Process and I get a "Terminate failed Dialog"
destroy() - before hostShell.exit();
java.io.IOException: Broken pipe
destroy() - after hostShell.exit();
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true
exitValue() - hostShell.isActive()=true

So the remote shell reports isActive correctly, only Local does it wrong.

(For the report with  RSE 1.0RC1 see https://bugs.eclipse.org/bugs/show_bug.cgi?id=158786#c7)

I don't know if this problem affects different Platform/OS, so I picked "All".
Feel free to change that if necessary.
Comment 1 Lothar Werzinger CLA 2006-10-21 11:38:54 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.
Comment 2 Martin Oberhuber CLA 2006-10-23 01:01:15 EDT
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
Comment 3 Lothar Werzinger CLA 2006-10-23 02:37:33 EDT
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.
Comment 4 Martin Oberhuber CLA 2006-10-24 10:33:11 EDT
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.
Comment 5 Lothar Werzinger CLA 2006-10-25 01:36:00 EDT
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.
Comment 6 Martin Oberhuber CLA 2006-10-25 03:23:29 EDT
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.
Comment 7 Lothar Werzinger CLA 2006-10-25 12:27:34 EDT
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.
Comment 9 Martin Oberhuber CLA 2007-05-18 15:50:34 EDT
Shooting for RC1.
Comment 10 Martin Oberhuber CLA 2007-08-03 10:40:59 EDT
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.