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

Corinna Vinschen corinna-cygwin@cygwin.com
Thu Aug 12 11:37:00 GMT 2010


On Aug 12 12:15, Andy Koppe wrote:
> On 12 August 2010 10:45, Corinna Vinschen wrote:
> > On Aug 12 10:36, Corinna Vinschen wrote:
> >> I had an overnight idea which I'll test today.  Basically, Cygwin still
> >> has the code to access the PEB directly.  So, what I intend to try is this:
> >>
> >>
> >>   bool invalid_path = false;
> >>   if incoming path is a virtual Cygwin dir
> >>     invalid_path = true;
> >>   else
> >>     invalid_path = !SetCurrentDirectoryW (incoming path);
> >>   if (invalid_path)
> >>     {
> >>       InterlockedExchange (PEB CWD handle, NULL);
> >>       NtClose (exchanged handle);
> >>       PEB CWD pathname = "---invalid path---";
> >>     }
> >>
> >> If that works as expected, this looks like an easy solution to me to
> >> disallow further path-relative WIn32 commands, if the CWD can't be
> >> handled by the Win32 API.
> >>
> >> I'll report back what I find.
> >
> > Oh, darn!  I just stumbled over the *other* comment in cwdstuff::set,
> > which points to another problem with SetCurrentDirectory:
> >
> >  Unlinking a cwd fails because SetCurrentDirectory seems to
> >  open directories so that deleting the directory is disallowed.
> >  The below code opens with *all* sharing flags set.
> >
> > So, actually, if we still want to be able to delete a cwd under the nose
> > of another Cygwin process. we don't want to call SetCurrentDirectory at
> > all!
> 
> Perhaps that's the way to go then. As a nice side effect, it would
> remove overhead that's unnecessary for the 99.99% of Cygwin programs
> that don't mix chdir() and CreateFile(). And it makes sense too that
> you'd need to use SetCurrentDirectory() with CreateFile(). The Win32
> working directory would still need to be set appropriately when
> invoking non-Cygwin programs.

Or don't start the Win32 app at all if the path is not valid.
However we have to call SetCurrentDirectory at least once, for the
first Cygwin app, into a spot it doesn't block the aforementioned
rmdir.  C:\ comes to mind.  It's the one directory which always
exists and it's not removable anyway.

> The argument against of course if backward compatibility, i.e. such a
> change would break programs that do depend on chdir() setting the
> Win32 working directory. On the other hand, they're already partially
> broken anyway for POSIX directories that don't have Win32 equivalents,
> so making that brokenness more obvious might actually be a good thing.

Right,


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat



More information about the Cygwin-developers mailing list