Closed
Bug 1004313
Opened 10 years ago
Closed 10 years ago
nsBufferedOutputStream::WriteSegments doesn't handle non-blocking streams well
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: jimb, Assigned: jimb)
References
Details
Attachments
(1 file)
12.90 KB,
patch
|
mcmanus
:
review+
|
Details | Diff | Splinter Review |
nsBufferedOutputStream::WriteSegments can return NS_ERROR_FAILURE even after having transferred some data to the underlying stream, causing the partial write count to be unavailable to JS callers, which makes it extremely difficult to use correctly from JS. The general convention for non-blocking I/O is that, if the operation was able to transfer any bytes at all, even if not as many as was requested, it returns the count and NS_OK. Only if the operation was unable to transfer any bytes whatsoever does it return NS_BASE_STREAM_WOULD_BLOCK. We can see this rule implemented correctly in nsBufferedOutputStream::Write: if a call to Flush fails, then we reach this exit path: *result = written; return (written > 0) ? NS_OK : rv; However, nsBOS::WriteSegments isn't so civil: rv = Flush(); if (NS_FAILED(rv)) return rv; As a consequence, if we go around the WriteSegments loop successfully once, and then a call to Flush fails, WriteSegments will return an error code, and the caller has no idea how many bytes were written. (I guess they can compare 'tell' results before and after, but that's horrible.)
Assignee | ||
Comment 1•10 years ago
|
||
Fix, including tests. (This patch also fixes a memcpy vs. memmove issue.)
Assignee | ||
Comment 2•10 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=38504c8d5ac6
Comment 3•10 years ago
|
||
Comment on attachment 8415679 [details] [diff] [review] Handle partial flushes correctly in nsBufferedOutputStream::WriteSegments. Review of attachment 8415679 [details] [diff] [review]: ----------------------------------------------------------------- thanks
Attachment #8415679 -
Flags: review?(mcmanus) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e0c131fc557
Flags: in-testsuite+
Target Milestone: --- → mozilla32
https://hg.mozilla.org/mozilla-central/rev/4e0c131fc557
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•