X-Recipient: archive-cygwin AT delorie DOT com
X-Spam-Check-By: sourceware.org
Date: Wed, 11 Aug 2010 10:49:26 +0200
From: Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
Message-ID: <20100811084926.GC26152@calimero.vinschen.de>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
References: <3C031C390CBF1E4A8CE1F74DE7ECAF3A140684F0AA AT MBX8 DOT EXCHPROD DOT USA DOT NET>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <3C031C390CBF1E4A8CE1F74DE7ECAF3A140684F0AA@MBX8.EXCHPROD.USA.NET>
User-Agent: Mutt/1.5.20 (2009-06-14)
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Precedence: bulk
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 Aug 10 21:53, John Carey wrote:
> On Aug 10 13:44 +0200 Corinna Vinschen wrote:
> >   chdir ("/");
> >   h = CreateFile ("foobar", ...);
> >   if (h == INVALID_HANDLE_VALUE) printf ("%lu\n", GetLastError());
> 
> Thanks for the test case for the CreateFile() problem; I used it to
> create the following test, in which Windows 7 CreateFile() fails with
> ERROR_INVALID_HANDLE even when using a stock Cygwin 1.7.5 DLL:
> [...]

Thanks for the test case.

> The PEB lock in cwdstuff::set() can only prevent the race if other
> threads always hold the PEB lock while invoking system calls that
> allocate handles.  CreateFile() is pretty big, so I might have missed
> it, but I did not see CreateFile() holding the PEB lock while actually
> creating the new file handle.

No, it doesn't have to.  It only needs the PEB lock when accessing
the cwd in the PEB.  But ...

>   Instead, after actually creating the
> new file handle, it called _RtlReleaseRelativeName AT 4 on
> a reference-counted object that holds a copy of the current
> directory handle (see below for more about such reference-counted
> current directory objects).

... if that's correct it seems that the cwd from the PEB isn't even used
in Vista++.  That explains why just exchanging the handle value in
cwdstuff::set() only works fine in 2K3 and earlier systems.  Bummer.
I assume the idea was to speed up CreateFile by getting rid of a lock
per call.

> Now, where is Windows stuffing the extra copy of the directory handle?
> In those reference-counted heap-allocated directory objects.  Stepping
> through the machine code under Windows 7 I see SetCurrentDirectory()
> updating a global pointer ("ntdll!RtlpCurDirRef") to one such object,
> under the protection of a critical section ("ntdll!FastPebLock").
> Despite mentioning "Peb" in the name, neither global's address is
> computed using "FS:".  The global pointer points to a heap-allocated
> block that starts with a reference count followed by a copy of the
> directory handle value, and apparently includes other data as well.
> SetCurrentDirectory() swaps in a new such block, and decrements the
> counter on the old block, and if that reaches 0, closes the handle.
> Anyway, this block appears to be where the old current directory
> handle value persists past the changes made by cwdstuff::set().

...and that handle is used in subsequent calls and potentially is a
handle to another object in the meantime.

I do basically agree with cgf that it's your own problem if you use
Win32 calls in your Cygwin app.  OTOH, I see that this can trip up
potential handle problems in the DLL itself.

Unfortunately that means there's no Win32-safe way to set a new
directory handle as cwd in Vista and later anymore.  Since there's no
official API to set the cwd using a directory handle, there's no way to
set the Win32 cwd to a directory with restricted permissions.
This *is* frustrating.

I'll look into another solution.  Probably we will have to call
SetCurrentDirectory again and ignore any error.  I don't accept the
aforementioned restriction for POSIX calls.


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