Last Comment Bug 757946 - nsConverterOutputStream logs "WARNING: Flush() lost data!" at close if underlying stream is already closed
: nsConverterOutputStream logs "WARNING: Flush() lost data!" at close if underl...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla15
Assigned To: :Irving Reid (No longer working on Firefox)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-23 12:02 PDT by :Irving Reid (No longer working on Firefox)
Modified: 2012-05-25 08:35 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Don't write to the underlying stream if flush has nothing to write (1010 bytes, patch)
2012-05-23 12:07 PDT, :Irving Reid (No longer working on Firefox)
smontagu: review+
Details | Diff | Review

Description :Irving Reid (No longer working on Firefox) 2012-05-23 12:02:21 PDT
The warning is frightening but harmless; the sequence of events that brought this on are:

JS code creates an nsISafeOutputStream to write safely to a file, then wraps that stream in a nsIConverterOutputStream

Once all the bytes are written through the converter, the JS code calls ConverterOutputStream.flush() and then SafeOutputStream.finish() to close and rename the underlying file to its final name. The JS code *cannot* just call ConverterOutputStream.close(), because calling close() on the SafeOutputStream aborts the file save operation; the temporary is only moved into its final position if finish() is called.

So, the underlying OutputStream is closed but the ConverterOutputStream is not. Then, when JS GC destroys the ConverterOutputStream, the destructor calls close(), which calls flush(), which always tries to write() to the underlying OutputStream even if it doesn't have anything to write.

I'll attach a patch for that issue, but it would also be worth making nsIConverterOutputStream implement the nsISafeOutputStream API so that calling code could just finish() the ConverterOutputStream and have the right things happen all the way down.
Comment 1 :Irving Reid (No longer working on Firefox) 2012-05-23 12:07:23 PDT
Created attachment 626541 [details] [diff] [review]
Don't write to the underlying stream if flush has nothing to write
Comment 2 Daniel Holbert [:dholbert] 2012-05-24 10:52:20 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea68a0638211
Comment 3 Ed Morley [:emorley] 2012-05-25 08:35:29 PDT
https://hg.mozilla.org/mozilla-central/rev/ea68a0638211

Note You need to log in before you can comment on or make changes to this bug.