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

waitReadable should block #3929

Merged
merged 2 commits into from
May 26, 2016
Merged

Conversation

headius
Copy link
Member

@headius headius commented May 26, 2016

This attempts to fix some sporadic readpartial tests and specs in the suite, specifically in this case a failure in MRI's test_readpartial.rb in test_open_pipe. Our old old would never properly wait for the pipe to become readable and would keep spinning and trying to do nonblocking reads followed by waitReadable that never waited. As a result we could have interrupts happen at unexpected times, leaving IO in a weird state that probably caused sporadic close errors like in https://travis-ci.org/jruby/jruby/jobs/132959357

headius added 2 commits May 25, 2016 20:49
This may have been an oversight; 0 means don't wait on the select
at all, but I may have been confused thinking it meant wait
forever. Doing waitReadable but not waiting for it to be readable
seems obviously wrong.
@headius
Copy link
Member Author

headius commented May 26, 2016

Another example, from https://travis-ci.org/jruby/jruby/jobs/131738145

     [exec]   1) Error:
     [exec] TestReadPartial#test_with_stdio:
     [exec] Errno::EBADF: Bad file descriptor - No message available
     [exec]     org/jruby/RubyIO.java:1955:in `close'
     [exec]     /home/travis/build/jruby/jruby/test/mri/ruby/test_readpartial.rb:14:in `make_pipe'
     [exec]     /home/travis/build/jruby/jruby/test/mri/ruby/test_readpartial.rb:26:in `pipe'
     [exec]     /home/travis/build/jruby/jruby/test/mri/ruby/test_readpartial.rb:61:in `test_with_stdio'

@headius headius merged commit a65ad3a into jruby:master May 26, 2016
@headius headius deleted the waitreadable_blocking branch May 26, 2016 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant