mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.
Date Thu, 19 Apr 2018 23:43:17 GMT


> On April 19, 2018, 2:07 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Line 83 (original), 87-95 (patched)
> > <https://reviews.apache.org/r/66439/diff/2/?file=1993515#file1993515line87>
> >
> >     I'm a bit surprised this doesn't cause problems with tests like `ProtobufIOTest.Basic`
and `OperationStatusUpdateManagerTests`.
> >     
> >     Since you're dup-ing the FDs for each write, if the caller passes in the same
FD for multiple calls, the original FD will retain its offset (near the beginning of the file)
and should repeatedly overwrite the data each time `protobuf::write` is called.
> 
> Akash Gupta wrote:
>     `dup` creates another file *descriptor* to the same file *description*, which stores
the offset. So, if you read from a dup'd handle, it changes the offset of the original handle.
The behavior is the same on Windows and Linux.

Yup, thanks Akash.

`man 2 dup`:
>  After a successful return, the old and new file descriptors may be used interchangeably.
 They refer to the same open file description (see open(2)) and thus share file offset and
file status flags; for example, if the file offset is modified by using lseek(2) on one of
the file descriptors,  the  offset  is  also changed for the other.

`DuplicateHandle` on MSDN:
> The duplicate handle refers to the same object as the original handle. Therefore, any
changes to the object are reflected through both handles. For example, if you duplicate a
file handle, the current file position is always the same for both handles.
This explains why the tests are passing ;)


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66439/#review201562
-----------------------------------------------------------


On April 6, 2018, 4:16 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66439/
> -----------------------------------------------------------
> 
> (Updated April 6, 2018, 4:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and Michael
Park.
> 
> 
> Bugs: MESOS-8675
>     https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is another edge case where a third-party library (protobuf)
> requires a CRT integer file descriptor. Thus we duplicate the `int_fd`
> and then explicitly allocate via `crt()`, which requires that we also
> manually close it via `_close()`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/protobuf.hpp 2fa5072e3c62c487da0dccffdd38d2fa1a615dc0

> 
> 
> Diff: https://reviews.apache.org/r/66439/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message