X-Recipient: archive-cygwin AT delorie DOT com
X-SWARE-Spam-Status: No, hits=-1.1 required=5.0	tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,TW_YG
X-Spam-Check-By: sourceware.org
X-USANET-Received: from gateout01.mbox.net [127.0.0.1] by gateout01.mbox.net via mtad (C8.MAIN.3.65C) 	with ESMTP id 311oiNwLF6224Mo1; Tue, 14 Sep 2010 22:11:05 -0000
X-USANET-Source: 165.212.120.254 IN   aeolus AT electric-cloud DOT com s1hub1.EXCHPROD.USA.NET
X-USANET-MsgId: XID932oiNwLF0259Xo1
From: John Carey <aeolus AT electric-cloud DOT com>
To: "cygwin AT cygwin DOT com" <cygwin AT cygwin DOT com>
Date: Tue, 14 Sep 2010 22:09:32 +0000
Subject: RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to 	thread-unsafe code in cwdstuff::set
Message-ID: <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD8A@MBX8.EXCHPROD.USA.NET>
References: <20100812081151 DOT GT14202 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA702A AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100903073740 DOT GA1749 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA702D AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100904092626 DOT GE16534 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA704C AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100911103010 DOT GM16534 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD78 AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100914081225 DOT GE16534 AT calimero DOT vinschen DOT de> <20100914161138 DOT GG15121 AT calimero DOT vinschen DOT de>,<20100914161636 DOT GH15121 AT calimero DOT vinschen DOT de>
In-Reply-To: <20100914161636.GH15121@calimero.vinschen.de>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-IsSubscribed: yes
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

On Sep 14 09:11, Corinna Vinschen wrote:
> I applied the below patch to Cygwin CVS and it appears to work nicely.
> The only potential race I can think of is, if another thread of the same
> Cygwin process calls SetCurrentDirectory.  I'm inclined to let this
> situation slip through the cracks since SetCurrentDirectory will already
> mess up a mixed-mode Cygwin process.
>
> The "wincap.has_transactions ()" is just for testing.  If we can use
> that code, I would replace is with something like
> "wincap.has_messed_up_cwd_handling()" or so.
...

I would argue against introducing a race with threads that call
SetCurrentDirectory(), for at least the following reasons:

1. The race can be avoided without too much grief.  While
NTDLL.DLL may change, a code scanner could detect that
and fall back cleanly to the unlocked behavior.  If cygcheck
and/or a unit test were to run the scanner directly and report
failure, then there would be no need for cygwin1.dll itself to
warn on code scan failure--silence there could avoid
clutter on this mailing list on some future patch day.

2. If a program does violate the rule against calling
SetCurrentDirectory() directly (possibly because of
some third party DLL being involved), then it could be
very difficult to detect the source of the resulting problems.
Relative path conversions could show a discrepancy, but
if all path conversions are absolute, then the only symptoms
would be rare, bizarre failures of apparently-unrelated
operations involving file handles, and overwriting of memory
reallocated to other tasks.  I would like to spare others
the kind of sleuthing I've had to do.

3. The unknown.  In software in general and on
Windows in particular, I prefer to program defensively.
It's difficult enough to get multithreaded programming
right when following the usual guidelines; venturing
beyond them is asking for trouble.  If there is some
threading subtlety in there that we haven't spotted,
it could become a huge headache to diagnose when
it eventually shows up.  For example, maybe some
Win32 API call copies the current directory handle from
a VistaCwd instance, or even from the PEB under the
protection of a lock, and expects it to remain open
during its work.  How would we rule out such a scenario?

Anyway, one other detail:

Are races within a pure-Cygwin program are prevented by
"cwd_lock"?  I don't see it being locked yet, just initialized.
Perhaps the cwdstuff::set patch is not yet written?

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