Community
Participate
Working Groups
It's possible for RemoteCommandShellOperation to miss output when reading from the shell via IRemoteCommandShell.listOutput(). Instead, it should use IRemoteCommandShell.getOutputAt().
Created attachment 138830 [details] patch to use getOutputAt()
Martin, I don't suppose there's any change for this to make it into RSE 3.1. The change is low-risk since nothing within base-RSE uses RemoteCommandShellOperation.
Go get a reviewer, I'm reviewing as well. Let's see what we can do.
Xuan, could you review this? Thanks.
I don't see how your patch would solve the problem. The outputUpdated() method which you changed is called repeatedly on the main thread, and will work on the amount of output that's available at that particular time. In the meantime, another thread generates output. Your change only means that addidional output gets captured, which happens to be created during the (really short) time in which we perform parsing. The original code took a snapshot at the time the main thread invoked outputUpdated(). It seems to me that taking a snapshot is not necessarily wrong, since outputUpdated() will be called again later with the next chunk of data anyways. After all we remember the index where we stopped parsing the last time. Even with your changed code it could happen that additional output gets created and we miss it due to some race condition. Why do you think this is Major? How often is failure observed and what is the impact of failure on the user? Could you confirm with a test that your code actually works as expected? Again, this fix looks like an ideal candidate for a Unit test which exercises the function a 1000 times or so in order to get confidence that it actually works properly. Without such a Unit test, I don't think that changing this such late in the game is the right thing to do. I might be swayed with very clear arguments and a unit test. If you don't feel like trying to push this into 3.1 please set an appropriate target milestone.
After investigating this further, it turns out that my patch simply altered the timing so that my scenarios were working. The real difficulty I hit is with SshServiceCommandShell.shellOutputChanged(): ... if (!gotCommand && line.equals(_curCommand)) { gotCommand = true; continue; //ignore remote command echo } ... RemoteCommandShellOperation waits until it receives the echo of the command before it starts processing output (so that it can ignore shell initialization text). Because the ssh service swallows the command echo, the RemoteCommandShellOperation never gets a chance to process the output. I'm not sure how we can get around that. I marked the bug as major because it takes away functionality for console IO in an IBM launch scenario.
Martin, do you have any suggestions on how to deal with this use case of SshServiceCommandShell?
not at the moment.
Bulk moving 3.2.x bugs to 3.3
Bulk moving 3.3 deferred items to 3.3.1
This issue has been migrated to https://gitlab.eclipse.org/eclipse/tm/org.eclipse.tm/-/issues/529.
