delorie.com/archives/browse.cgi   search  
Mail Archives: cygwin/2012/07/17/04:06:28

X-Recipient: archive-cygwin AT delorie DOT com
X-Spam-Check-By: sourceware.org
Date: Tue, 17 Jul 2012 10:05:32 +0200
From: Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: pread vs. git
Message-ID: <20120717080532.GA27935@calimero.vinschen.de>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
References: <5004EF52 DOT 1000607 AT redhat DOT com>
MIME-Version: 1.0
In-Reply-To: <5004EF52.1000607@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Id: <cygwin.cygwin.com>
List-Unsubscribe: <mailto:cygwin-unsubscribe-archive-cygwin=delorie DOT com AT cygwin DOT com>
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com

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

- Raw text -


  webmaster     delorie software   privacy  
  Copyright © 2019   by DJ Delorie     Updated Jul 2019