X-Recipient: archive-cygwin@delorie.com X-Spam-Check-By: sourceware.org Date: Wed, 15 Sep 2010 12:08:28 +0200 From: Corinna Vinschen To: cygwin@cygwin.com Subject: Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set Message-ID: <20100915100828.GO15121@calimero.vinschen.de> Reply-To: cygwin@cygwin.com Mail-Followup-To: cygwin@cygwin.com References: <20100903073740.GA1749@calimero.vinschen.de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA702D@MBX8.EXCHPROD.USA.NET> <20100904092626.GE16534@calimero.vinschen.de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA704C@MBX8.EXCHPROD.USA.NET> <20100911103010.GM16534@calimero.vinschen.de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD78@MBX8.EXCHPROD.USA.NET> <20100914081225.GE16534@calimero.vinschen.de> <20100914161138.GG15121@calimero.vinschen.de> <20100914161636.GH15121@calimero.vinschen.de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD8A@MBX8.EXCHPROD.USA.NET> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD8A@MBX8.EXCHPROD.USA.NET> User-Agent: Mutt/1.5.20 (2009-06-14) Mailing-List: contact cygwin-help@cygwin.com; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-owner@cygwin.com Mail-Followup-To: cygwin@cygwin.com Delivered-To: mailing list cygwin@cygwin.com On Sep 14 22:09, John Carey wrote: > 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. Right. Still, the licensing issue... > 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. But we're talking about mixed-mode Cygwin apps. They should use the Win32 API with care anyway. And the disconnection between SetCurrentDirectory and chdir already exists if the latter is called at all. > [...] > 3. The unknown. Hard to argue aginst *that*... > 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? There's a cwd_lock.acquire and a cwd_lock.release call in cwdstuff::set. There's no such lock when cwdstuff::override_win32_cwd is called from cwdstuff::init, because at the time init is called there's no other concurrent thread running, except for the signal thread. 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