pread vs. git

[Corinna Vinschen]

On Jul 16 22:51, Eric Blake wrote:
> Recent git added a hack to explicitly avoid cygwin's pread as unsafe:
> 
> http://git.661346.n2.nabble.com/PATCH-v2-1-1-index-pack-Disable-threading-on-cygwin-td7562195.html

Boy, is that annoying.  Cygwin's pread/pwrite is thread-safe since June
2011.  The implementation isn't optimal, but at least the thread-safety
is ensured by the current implementation.  What version of Cygwin is
that guy using?

> Comments like this aren't very re-assuring either:
> 
> http://sourceware.org/ml/cygwin/2011-06/msg00057.html

This mail refers to a temporary patch which has been overridden by
the current implementation, described by the below comment.

> or this big hairy comment in the source of fhandle_disk_file:
> 
>    Using this handle for pread/pwrite would break atomicity, because the
>    read/write operation would have to be followed by a seek back to the old
>    file position.  What we do is to open another handle to the file on the
>    first call to either pread or pwrite.  This is used for any subsequent
>    pread/pwrite.  Thus the current file position of the "normal" file
>    handle is not touched.
> 
>    FIXME:
> 
>    Note that this is just a hack.  The problem with this approach is that
>    a change to the file permissions might disallow to open the file with
>    the required permissions to read or write.  This appears to be a border case,
>    but that's exactly what git does.  It creates the file for reading and
>    writing and after writing it, it chmods the file to read-only.  Then it
>    calls pread on the file to examine the content.  This works, but if git
>    would use the original handle to pwrite to the file, it would be broken
>    with our approach.
> 
>    One way to implement this is to open the pread/pwrite handle right at
>    file open time.  We would simply maintain two handles, which wouldn't
>    be much of a problem given how we do that for other fhandler types as
>    well.
> 
>    However, ultimately fhandler_disk_file should become a derived class of
>    fhandler_base_overlapped.  Each raw_read or raw_write would fetch the
>    actual file position, read/write from there, and then set the file
>    position again.  Fortunately, while the file position is not maintained
>    by the I/O manager, it can be fetched and set to a new value by all
>    processes holding a handle to that file object.  Pread and pwrite differ
>    from raw_read and raw_write just by not touching the current file pos.
> 
> 
> What still remains to make pread a first-class thread-safe
> implementation that obeys POSIX, so that I don't have to cripple my next
> build of git to avoid threaded pread?

The fix this comment refers to was specificially created to make git work.
It is thread-safe.  The downside of the current implementation is exactly
described, but is no problem for current git since it does *not* use the
original handle to call pwrite.  What's also described is how to fix this
problem once and for all.  http://cygwin.com/acronyms/#SHTDI


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple