Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error stream is never closed #27

Closed
alexeykazakov opened this issue Jun 15, 2016 · 25 comments
Closed

Error stream is never closed #27

alexeykazakov opened this issue Jun 15, 2016 · 25 comments

Comments

@alexeykazakov
Copy link

In devstudio we do "vagrant up" for cdk2.1-rc3/cdk/components/rhel/rhel-ose Vagrant file with vagrant-sshfs plugin installed.
Then we read output and error streams. Output stream is terminated eventually but error stream is always open. We do read "Warning: Permanently added '[127.0.0.1]:2222' (ECDSA) to the list of known hosts." from the error stream which looks like came from sshfs plugin.
devstudio is stuck because the error stream is never closed.

@alexeykazakov
Copy link
Author

This is a blocker for devstudio - https://issues.jboss.org/browse/JBIDE-22604

@jeffmaury
Copy link

We launch "vagrant up --no-color through the Java runtime. Then we launched 2 threads that read the output and error streams for the launched process and terminate when the streams are closed. Those 2 threads never stopped when the process object representing the vagrant process in the Java runtime return the exit code so vagrant process has stopped

@LalatenduMohanty
Copy link

@alexeykazakov can you please put steps to reproduce?

@jeffmaury
Copy link

@LalatenduMohanty If you look at the JIRA there is a link to a Java prog that will launch the CDK the same way DevStudio does. If fails starting from RC2

@alexeykazakov
Copy link
Author

alexeykazakov commented Jun 15, 2016

Yes the easiest way to reproduce is to use @jeffmaury's java prog: https://gist.github.com/jeffmaury/ba61c91fe910220585e9893540ccec7d

Args are:
/usr/bin/vagrant /path_to_cdk_rc2_or_rc3/cdk/components/rhel/rhel-ose [email protected] password

The program is never ended. It waits for error thread die.

@robstryker
Copy link

robstryker commented Jun 15, 2016

#version stuff
vagrant version | head -n 2
vboxmanage --version

# get pre-reqs
wget http://cdk-builds.usersys.redhat.com/builds/weekly/14-Jun-2016.rc3/cdk.zip
unzip cdk.zip
wget http://cdk-builds.usersys.redhat.com/builds/weekly/14-Jun-2016.rc3/rhel-cdk-kubernetes-7.2-25.x86_64.vagrant-virtualbox.box

# get and compile test java class
curl https://gist.githubusercontent.com/jeffmaury/ba61c91fe910220585e9893540ccec7d/raw/504389c35a5074e788598668c44bb377dbf948e6/VagrantLauncher.java | tail -n +2 > VagranLauncher.java
javac VagrantLauncher.java

# add box
vagrant box add --name cdkv2 rhel-cdk-kubernetes-7.2-25.x86_64.vagrant-virtualbox.box

# install plugin
vagrant plugin install vagrant-sshfs


folder=`pwd`
vagr=`which vagrant`
user1="[email protected]"
pass1="hunter2"
java VagrantLauncher $vagr $folder/cdk/components/rhel/rhel-ose $user1 $pass1

@robstryker
Copy link

Here are the versions i can replicate on, but 2 others have replicated nad I can't speak for their versions.

[rob@rawbdor 20160614]$ vagrant version | head -n 2
Installed Version: 1.7.2
Latest Version: 1.8.4
[rob@rawbdor 20160614]$ vboxmanage --version
4.3.36r105129

@LalatenduMohanty
Copy link

I could reproduce the issue :(

dustymabe added a commit that referenced this issue Jun 15, 2016
Otherwise they stay open and some "wrapping" processes can't handle
that. Fixes #27
@LalatenduMohanty
Copy link

LalatenduMohanty commented Jun 16, 2016

Looks like the fix is not working for @jeffmaury on Windows 10

hferentschik pushed a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 16, 2016
@hferentschik
Copy link
Contributor

It waits for error thread die.

For what it's worth. I does not look like that the error threadd/io is the (only) problem. The stdout thread hangs as well. I tried with various process group flags to spawn as well as trying to flush the streams on the Ruby side. So far no success.

@hferentschik
Copy link
Contributor

The problem comes in when you read the IO via threads. The input stream is blocked on a read. If I change the Java code to redirect the in- and output to a file as described in this post - http://java-monitor.com/forum/showthread.php?t=4067 - it works fine. This is probably not a solution suitable for us, since I cannot display the content of the executed command as it occurs, but it shows that it is generally possible. My guess is that one could rewrite the logic in the IO reader threads to use non blocking nio. This of course requires that we have control over this part of the code. I am not saying that there is no issue/bug in the Ruby code, but for all I can tell it looks ok (I also tried several things w/o avail).

@alexeykazakov
Copy link
Author

alexeykazakov commented Jun 16, 2016

From #cdk in IRC:

jmaury: got the pb reproduced on windows with a simple windows shell !!!!
akazakov: jmaury, you mean you reproduced the problem even in cli on windows?
jmaury: yes
jmaury: 1) open a windows shell 2) go to vagrantfile dir 3) vagrant up 4) exit --> shell window won't close 5) kill ssh.exe process launched --> shell window will now close

So, it looks like a bug in the Ruby code or somewhere else upstream. We could probably work around it somehow in the java code on devstudio but it will make sense only if we can't solve it in a lower level.

@alexeykazakov
Copy link
Author

Shouldn't we re-open this issue?

@jeffmaury
Copy link

Yes, I think so. Working on a simple use case

@hferentschik
Copy link
Contributor

It might be a windows thing and how it handles files, pipes, etc. It seems like the parent process cannot exit while this pipe created to connected the two spawned processes exist. Maybe this is just a limitation/feature of Windows. Unfortunately, I keep drawing blanks when trying to search for this type of issue.

Working on a simple use case

From the Ruby side? This was my plan as well. Just a simple Ruby script which spawns two processes and ties them together with a pipe. No Vagrant, sftp, ssh, etc. I would expect this test script to behave the same way as the vagrant-sshfs plugin. @jeffmaury Do you have already such a test script?

@hferentschik
Copy link
Contributor

Shouldn't we re-open this issue?

I suggest we open a new one.

@hferentschik
Copy link
Contributor

It is even worse than I thought. This issue can be re-produced with a single spawn as well:

p1 = spawn('ruby', 'loop.rb', 'p1', [:out, :err] => ['process.log', "w"],  :new_pgroup => true)

For a full working example see this gist - https://gist.github.com/hferentschik/f94a13a81f2f6e1be3e48d9c59e302c3

You can run ruby spawner.rb which will spawn another Ruby program (loop.rb) which will just run in an endless loop. Executing spawner.rb will return and there will be a new process for loop.rb. On OS X I can now close the terminal and the loop process continues. On Windows when I close the shell I launched the process from, the child process dies. This despite even specifying new_pgroup => true which gives the new process its own process group.

@jeffmaury
Copy link

There is a SubProcess gem that seems to use Win32 API CreateProcess. Maybe worth trying

@hferentschik
Copy link
Contributor

@jeffmaury I have investigated several process generation gems (I have the same issue with the Landrush plugin). They all broke down somewhere. In case you mean this gem - https://github.com/stripe/subprocess -, then I don't think it works. AFAICT it is based on fork.

There is https://github.com/djberg96/win32-process/ which indeed makes win32 API calls. I am pretty sure I looked at this gem before, but cannot remember why I went for the spawn approach. Most likely since spawn seemed simpler and seemingly worked. It might be worth a shot.

@dustymabe
Copy link
Owner

TBH - The only thing I know for sure is that vagrant-sshfs works reliably when run in a cygwin terminal. Can you execute all of your tests from within a cygwin terminal (if that even makes sense) and let us know the difference in the results?

@jeffmaury
Copy link

No I meant childprocess because it's the one used by vagrant

Jeff
Le 17 juin 2016 14:32, "Hardy Ferentschik" [email protected] a
écrit :

@jeffmaury https://github.com/jeffmaury I have investigated several
process generation gems (I have the same issue with the Landrush plugin).
They all broke down somewhere. In case you mean this gem -
https://github.com/stripe/subprocess -, then I don't think it works.
AFAICT it is based on fork.

There is https://github.com/djberg96/win32-process/ which indeed makes
win32 API calls. I am pretty sure I looked at this gem before, but cannot
remember why I went for the spawn approach. Most likely since spawn seemed
simpler and seemingly worked. It might be worth a shot.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#27 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAqeuZpmJ9-2HoKbLcZE7zLB0o9SWs8Mks5qMpOwgaJpZM4I2u0A
.

@hferentschik
Copy link
Contributor

@jeffmaury childprocess is not working. This is the first thing I tried ways back - enkessler/childprocess#99

@hferentschik
Copy link
Contributor

Can you execute all of your tests from within a cygwin terminal

Same problem

@dustymabe
Copy link
Owner

got ya - for posterity hardy asked a question on stackoverflow here:

http://stackoverflow.com/questions/37881385/spawned-ruby-process-on-windows-dies-when-shell-is-terminated

@hferentschik
Copy link
Contributor

The win32-process gem seems to be the solution. I tried it with the test setup (see gist) and it worked using the shell (I could close it and the process kept running) and it worked with the Java launcher as well. For all I can tell Ruby's impl of the spawn method must be doing something wrong and the way to do it is via native win31 api calls. Now we need to change the code for vagrant-sshfs and landrush :-)

hferentschik pushed a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 20, 2016
hferentschik added a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
hferentschik pushed a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
hferentschik added a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
hferentschik pushed a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
hferentschik added a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
hferentschik added a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
hferentschik added a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
hferentschik added a commit to hferentschik/vagrant-sshfs that referenced this issue Jun 21, 2016
dustymabe pushed a commit that referenced this issue Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants