kidok / protobuf

Automatically exported from code.google.com/p/protobuf
0 stars 0 forks source link

OstreamOutputStream with ofstream loses data from the tail #592

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Use an OstreamOutputStream with an ofstream as the underlying IO stream.
2. Write data that is not aligned to the buffer boundary for the ofstream 
implementation.  This happens surprisingly often for me (e.g. try writing 
somewhere between 8192 and 8192+127 bytes).  It happens even with fewer bytes.
3. Alternatively, the attached patch adds a test to 
zero_copy_stream_unittest.cc which fails without the attached fix.

What is the expected output? What do you see instead?
Expected output is that OstreamOutputStream should work regardless of the 
amount of data written.  The attached fix addresses this issue by adding a 
Flush operation to the underlying implementation.

Please use labels and text to provide additional information.
Protobuf version 2.5.0
gcc version used was 4.8.1, clang version 3.0

Original issue reported on code.google.com by abhishek...@gmail.com on 2 Jan 2014 at 7:47

Attachments:

GoogleCodeExporter commented 9 years ago
I think the current implementation is working as expected. The problem you 
described is the result of not closing the file properly (where flush() will be 
called on the ostream) after writing. In the test you added in the attached 
patch, close() should be called before reopening the file for reading.

Original comment by xiaof...@google.com on 10 Jan 2014 at 12:19

GoogleCodeExporter commented 9 years ago
I mostly agree and don't think it's a big deal either way.  In my usage, the 
ostream was getting closed much later, hence I ran into the issue.  However, I 
still think it will be good if OstreamOutputStream, on its exit path, commits 
its writes to the ostream by means of a flush.

Thanks,
Abhishek Rai

Original comment by abhishek...@gmail.com on 10 Jan 2014 at 12:36

GoogleCodeExporter commented 9 years ago
Only the owner of the ostream knows when is the most appropriate time to call 
flush() and apparently OstreamOutputStream doesn't own the underlying ostream 
so it's not its responsibility to flush the stream. 
Imaging the case where you need to serialize a lot of messages into the same 
file, would you like to have every proto.SerializeToOstream(out) call flush the 
out stream?

Original comment by xiaof...@google.com on 10 Jan 2014 at 12:49

GoogleCodeExporter commented 9 years ago
Sounds good, thanks!

Thanks,
Abhishek Rai

Original comment by abhishek...@gmail.com on 10 Jan 2014 at 1:03