1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set

John Carey aeolus@electric-cloud.com
Wed Sep 15 02:44:00 GMT 2010


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



More information about the Cygwin mailing list