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

[fix][backport] Ensure copy_stream write sends all bytes read #6348

Merged
merged 1 commit into from Jul 30, 2020

Conversation

kares
Copy link
Member

@kares kares commented Jul 30, 2020

On target streams that may do partial reads, the logic here fails
to write all content read from the source stream. The return value
of Channel.write is never checked, but the total bytes written is
incremented, resulting in silently losing data.

The patch loops until all bytes have been written to the output
channel after each read.

CRuby does similar logic. We may need to add additional blocking
write or thread event checks here, in case the target stream
blocks during the write loop, but that will require the different
transfer paths to be expanded to support nonblocking IO
channels.

Fixes #6078

On target streams that may do partial reads, the logic here fails
to write all content read from the source stream. The return value
of Channel.write is never checked, but the total bytes written is
incremented, resulting in silently losing data.

The patch loops until all bytes have been written to the output
channel after each read.

CRuby does similar logic. We may need to add additional blocking
write or thread event checks here, in case the target stream
blocks during the write loop, but that will require the different
`transfer` paths to be expanded to support nonblocking IO
channels.

Fixes jruby#6078
@kares
Copy link
Member Author

kares commented Jul 30, 2020

was thinking this one is rather small and low risk ... maybe it doesn't have to wait till 9.3

@kares kares added this to the 9.2.13.0 milestone Jul 30, 2020
@kares kares merged commit 36084b5 into jruby:jruby-9.2 Jul 30, 2020
1 check passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants