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

Timeout::timeout not throwing exception when timeout period reaches #1050

Closed
navaneeth opened this issue Sep 30, 2013 · 12 comments
Closed

Timeout::timeout not throwing exception when timeout period reaches #1050

navaneeth opened this issue Sep 30, 2013 · 12 comments
Labels
Milestone

Comments

@navaneeth
Copy link

@navaneeth navaneeth commented Sep 30, 2013

/tmp/loop.sh

#!/bin/sh
while :
do
    sleep 1
done

Now start the above script from a Ruby script.

/tmp/test.rb

require 'timeout'
begin
    Timeout::timeout(1) {
        `/tmp/loop.sh`.chomp
    }
rescue Exception => e
    puts 'inside rescue'
rescue Error => e1
    puts "inside error"
end

Execute the above script in a JRuby ScriptingContainer.

ScriptingContainer container = new ScriptingContainer();
container.runScriptlet(PathType.ABSOLUTE, "/tmp/test.rb");

The rescue block is not getting executed. Tested this with MRI and it works well there.

@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Oct 1, 2013

This is a known limitation with JRuby subprocess launching right now. The process management on JVM does not allow cleanly interrupting a blocking IO read of the subprocess, and as a result we can't cause it to timeout.

We plan to fix this by implementing our own subprocess logic, but unfortunately for now this is a limitation.

@navaneeth

This comment has been minimized.

Copy link
Author

@navaneeth navaneeth commented Oct 1, 2013

@headius Thanks. I'd like to work on this. Do you have some blueprint/wiki which explains what needs to be done for this feature?

@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Oct 9, 2013

We have a number of folks interested...the key task is to start getting an FFI or jnr-ffi (our Java equivalent) version of process launching to do everything Ruby wants. See projects like posix_spawn for examples.

@rtyler

This comment has been minimized.

Copy link

@rtyler rtyler commented Aug 7, 2015

FWIW, I've reproduced that this is still the case with JRuby 9.0.0.0 whereas MRI 2.1.5 puts "inside rescue"

@Adithya-copart

This comment has been minimized.

Copy link

@Adithya-copart Adithya-copart commented Aug 13, 2019

I faced the same issue when I tried to do something like this in 9.2.8.0:

require 'timeout'
begin
  Timeout.timeout(5) do
    pid = spawn('ruby -e "sleep"')
    Process.wait pid
  end
rescue Timeout::Error => e
  'timed out'
end
@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Aug 22, 2019

I suspect MRI is using a non-blocking form of wait here, that allows them to give up on the subprocess. I will look into whether we can do the same.

@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Aug 27, 2019

After looking into this I think the cleanest we could support interruptible Process.wait would be to have it poll. CRuby appears to just go straight into waitpid and uses pthread_kill to interrupt it. We could probably acquire the native thread ID and do the same, but I worry what effect it might have on the JVM itself.

@Adithya-copart

This comment has been minimized.

Copy link

@Adithya-copart Adithya-copart commented Aug 27, 2019

I used somewhat of a similar approach here as a work-around. I called Process.wait in a separate thread and handled the necessary timeout logic based on polling Thread.status.

I don’t know the merits or de-merits of the suggested approach but at-least JRuby provides a way to implement this. I use caution using Timeout after reading your 10-year old blog post recently even though I don’t know if your recommendation changed after that.

@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Aug 27, 2019

So it turns out that the JVM also uses pthread_kill when interrupting, among other things, blocking IO calls. I've got a patch in progress that we can evaluate.

@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Aug 27, 2019

@Adithya-copart Yes, that works...or if you can afford to do some polling with a timeout, you can waitpid with the WNOHANG flag, which returns immediately.

I will push a PR you can play with.

headius added a commit to headius/jruby that referenced this issue Aug 27, 2019
This addresses jruby#1050 in the same way CRuby implements this. In
CRuby, the wait functions are called directly, and the
`pthread_kill` function is used to force the call to be
interrupted. It turns out the JVM also uses `pthread_kill` to
interrupt blocking native calls, so we use the same binding they
do.

Note that this will only be used on non-Windows, but the JDK's
binding of `NativeThread.signal` might already be stubbed out
there.
@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Aug 27, 2019

I've pushed #5848, which wraps the wait, waitpid and waitall blocking calls with logic that uses pthread_kill to force an interrupt. It appears to pass the case given in #1050 (comment).

This is not tied into backticks, so it does not yet fix the original reported issue.

headius added a commit to headius/jruby that referenced this issue Aug 27, 2019
This addresses jruby#1050 in the same way CRuby implements this. In
CRuby, the wait functions are called directly, and the
`pthread_kill` function is used to force the call to be
interrupted. It turns out the JVM also uses `pthread_kill` to
interrupt blocking native calls, so we use the same binding they
do.

Note that this will only be used on non-Windows, but the JDK's
binding of `NativeThread.signal` might already be stubbed out
there.
@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Aug 29, 2019

Fixed by #5848.

@headius headius closed this Aug 29, 2019
@headius headius added this to the JRuby 9.2.9.0 milestone Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.