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

IO#seek and tell do not properly handle offsets larger than int32 #3817

Closed
headius opened this Issue Apr 20, 2016 · 6 comments

Comments

Projects
None yet
4 participants
@headius
Member

headius commented Apr 20, 2016

Environment

JRuby 9.1

Expected Behavior

IO#seek and IO#tell should handle offsets larger than int32 range

Actual Behavior

As seen in #3435, we were not handling the return value of lseek properly, interpreting all negative values as error rather than just -1. That was fixed, but the larger issue is that our native lseek and much of the code that calls it only supports int32 range.

I have a commit coming shortly to fix the main #3435 issue, but this large problem needs to be addressed after 9.1.

@headius headius added this to the JRuby 9.1.1.0 milestone Apr 20, 2016

headius added a commit that referenced this issue Apr 20, 2016

Only treate -1 ret from lseek as error. Fixes #3435.
There's a larger bug here, in that our native lseek does not
return a 64-bit value. This results in seeks past the range of a
32-bit int overflowing to negative, which triggered the errors in

See #3817 for remaining work.
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 20, 2016

Member

See #3664 for another report partially fixed by #3435.

See jnr/jnr-posix#71 for a PR to fix at least some of these APIs in jnr-posix.

Member

headius commented Apr 20, 2016

See #3664 for another report partially fixed by #3435.

See jnr/jnr-posix#71 for a PR to fix at least some of these APIs in jnr-posix.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Sep 29, 2016

Member

I had some trouble figuring out how to test this.

Member

headius commented Sep 29, 2016

I had some trouble figuring out how to test this.

@byteit101

This comment has been minimized.

Show comment
Hide comment
@byteit101

byteit101 Sep 29, 2016

Member

I had a small (well, not file size small, but code small) test case when I reported this on #3664 that could be potentially adapted as a unit test provided you have 3G/5G of disk space

Member

byteit101 commented Sep 29, 2016

I had a small (well, not file size small, but code small) test case when I reported this on #3664 that could be potentially adapted as a unit test provided you have 3G/5G of disk space

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Sep 29, 2016

Member

@byteit101 Yeah it's the file size I'm concerned about. I was able to use /dev/zero to test the jnr-posix fix, but that seems to blow up for me with ruby.

Member

headius commented Sep 29, 2016

@byteit101 Yeah it's the file size I'm concerned about. I was able to use /dev/zero to test the jnr-posix fix, but that seems to blow up for me with ruby.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Sep 29, 2016

Member

Oh nevermind...I had reversed seek params. Test coming.

Member

headius commented Sep 29, 2016

Oh nevermind...I had reversed seek params. Test coming.

headius added a commit that referenced this issue Sep 29, 2016

@headius headius removed the needs tests label Sep 29, 2016

eregon added a commit to ruby/spec that referenced this issue Oct 29, 2016

@Goltergaul

This comment has been minimized.

Show comment
Hide comment
@Goltergaul

Goltergaul Aug 18, 2017

Hi is this issue fixed now on newer jrubies?

Goltergaul commented Aug 18, 2017

Hi is this issue fixed now on newer jrubies?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment