X-Recipient: archive-cygwin AT delorie DOT com
X-SWARE-Spam-Status: No, hits=0.8 required=5.0	tests=BAYES_50,RCVD_IN_DNSWL_NONE
X-Spam-Check-By: sourceware.org
X-USANET-Source: 165.212.120.254 IN   aeolus DOT electric-cloud DOT com AT electric-cloud DOT usa DOT net s1hub3.EXCHPROD.USA.NET
X-USANET-MsgId: XID182oHJaVU4731Xo2
From: John Carey <aeolus DOT electric-cloud DOT com AT electric-cloud DOT usa DOT net>
To: "cygwin AT cygwin DOT com" <cygwin AT cygwin DOT com>
Date: Tue, 10 Aug 2010 00:19:54 +0000
Subject: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
Message-ID: <3C031C390CBF1E4A8CE1F74DE7ECAF3A140684F0A2@MBX8.EXCHPROD.USA.NET>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Id: <cygwin.cygwin.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

After a call to SetCurrentDirectory(), I have seen occasional (< 5%) failur=
es of CreatePipe() with code ERROR_INVALID_HANDLE.  I have also seen failur=
e of Cygwin signal handling because the signal handling pipe has mysterious=
ly closed.

I believe that both symptoms are due to this code in winsup/cygwin/path.cc,=
 which as the comments state, is not thread-safe:

      /* Workaround a problem in Vista/Longhorn which fails in subsequent
         calls to CreateFile with ERROR_INVALID_HANDLE if the handle in
         CurrentDirectoryHandle changes without calling SetCurrentDirectory,
         and the filename given to CreateFile is a relative path.  It looks
         like Vista stores a copy of the CWD handle in some other undocumen=
ted
         place.  The NtClose/DuplicateHandle reuses the original handle for
         the copy of the new handle and the next CreateFile works.
         Note that this is not thread-safe (yet?) */
      NtClose (*phdl);
      if (DuplicateHandle (GetCurrentProcess (), h, GetCurrentProcess (), p=
hdl,
                           0, TRUE, DUPLICATE_SAME_ACCESS))
        NtClose (h);

If another thread allocates a handle between the NtClose and the Duplicate,=
 then the handle value is not preserved, and strange effects may result.  P=
resumably the issues mentioned in the source comment may also occur, but wh=
at I have seen myself is a subsequent call to SetCurrentDirectory() closing=
, not the current directory handle, but the handle that was allocated by th=
e other thread.

Specifically, dll_crt0_1() calls sigproc_init(), then cwdstuff::set(), and =
finally wait_for_sigthread().  The function sigproc_init() creates the sign=
al handling thread, which creates the signal handling pipe as
one of its very first actions, then sets the event for which wait_for_sigth=
read() waits.  I think this scenario happens:

  1. main thread: cwdstuff::set(): NtClose().

  2. signal handling thread: CreatePipe() opens a handle to '\Device\NamedP=
ipe\' and stores it in a global variable (because this is the first call to=
 CreatePipe()).

  3. main thread: cwdstuff::set(): DuplicateHandle().

In this case, the current directory handle value has changed, which is not =
the intend of the NtClose-Duplicate sequence.  Perhaps it causes CreateFile=
 to fail with ERROR_INVALID_HANDLE as mentioned in the source comments, but=
 I have not seen that.  I think that the CreatePipe() failures I have seen =
are triggered when SetCurrentDirectory() closes the handle to '\Device\Name=
dPipe\', thinking that it is the current directory handle.  After that, Cre=
atePipe() will fail with ERROR_INVALID_HANDLE.

I think this other scenario also happens:

  1. signal handling thread: CreatePipe() opens a handle to '\Device\NamedP=
ipe\' (because it is the first call to CreatePipe()).

  2. main thread: cwdstuff::set(): NtClose().

  3. signal handling thread: CreatePipe() opens pipe handles.

  4. main thread: cwdstuff::set(): DuplicateHandle().

In this case it is Cygwin signal handling that is sabotaged by subsequent c=
alls to SetCurrentDirectory(), because they close one of the pipe handles u=
sed for Cygwin signal handling.

Note that replacing calls to SetCurrentDirectory() with chdir() could actua=
lly make the problem worse, because it would trigger the thread-unsafe code=
 more frequently.  The call to SetCurrentDirectory() is merely making it po=
ssible to detect the problem sooner.

Though this would not eliminate the problem entirely, would it be possible =
to better synchronize the signal handling thread during Cygwin initializati=
on, either by delaying creation of that thread until the first cwdstuff::se=
t(), or by calling wait_for_sigthread() before cwdstuff::set()?

-- John

--
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