Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 14 22:11, John Carey wrote: On Sep 14 12:02, Corinna Vinschen wrote: True. Implementing a full replacement for SetCurrentDirectory as in your PoC is still an option. However, I can't do that anymore since I'm tainted by reading your code. If you would contemplate to sign a copyright assignment(**), we could create a more thorough solution with code scanning and all that in the long run. In the meantime, I could apply my patch and we can try how well it works, hacked as it is. Sorry, I had not realized that I would cause a problem by supplying proof of concept code. I'll inquire... I'm looking forward. 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 13 19:47, John Carey wrote: On Sep 11 03:30, Corinna Vinschen: Given that, wouldn't it be potentially possible to override the content of curCwd? The problem is probably (just as in my old Cygwin code up to 1.7.5) to make sure that this is thread safe. Probably we would still need CwdCS. Unfortunately, it looks as if Win32 API functions guarantee to each other that they will not modify the path in a VistaCwd whose reference count is = 2. (Though it looks like they may update OldDismountCount under a CwdCS lock.) Consider RtlGetCurrentDirectory_U(). It shares a (non-exported) subroutine with other Win32 API functions that acquires a counted reference to the current VistaCwd instance under the protection of a lock on CwdCS. (This subroutine also does some kind of freshness check against DismountCount.) But that subroutine releases its lock before returning, and RtlGetCurrentDirectory_U() then uses the pathname in the returned VistaCwd instance *without* holding a lock. Specifically, it copies that pathname to a memory buffer passed in by its own caller. Pathname copies are not atomic. Thus, unless there is a bug in RtlGetCurrentDirectory_U(), it has some guarantee that other threads will not modify the pathname in its VistaCwd instance, though they are free to allocate a new VistaCwd instance and assign its address to the global Cwd pointer (so long as they lock CwdCS). Presumably the point of the Vista++ CWD mechanism is to reduce contention on the CWD lock. Threads acquire that lock for very short periods of time. They appear to avoid making system calls or even allocating memory while holding it. Also, the CWD lock is no longer the PEB lock, so CWD usage does not serialize with other PEB-related activities. Anyway, it looks to me as if overwriting the path in an existing VistaCwd instance would confuse RtlGetCurrentDirectory_U() and probably other Win32 API functions. (It may be that the same is true for the handle member; I have not tried to find that out.) Cygwin should probably allocate a new VistaCwd instance, as does the Win32 SetCurrentDirectory() implementation. Hmm, I'm still wondering. In theory we don't have to replace the directory name. We just have to InterlockedExchange the handle, since we only need to replace the original handle which does not allow sharing-for-delete with our own handle which does. All other border cases (virtual dir, directory with restricted rights) can be rightfully handled by setting the Win32 CWD to \\?\PIPE\ again. That's at least worth a try, isn't it? == critical section endl; cout showbase hex (size_t)Cwd == Vista++ CWD struct pointer endl; Is there any connection between these two addresses, like, say, they are side by side? Not that I have been able to find. The gap between Cwd and CwdCS is always positive, but is large and varies among OS versions: Bummer. I was hoping that these global variables could be identified as part of some other global structure for which there is an easier way to find it's address. Can't we find Cwd by scanning the .data segment of ntdll.h for the address we inferred from the Params.CurrentDirectoryName.Buffer value? Nice; that might work. But there would need to be some kind of rule to pick a winner among multiple matching words, in case by coincidence some other non-pointer word just happens to have the same bit pattern. I can see two alternatives for the multiple-match case: 1. Code scanning, as suggested earlier. There would need to be a unit test of the code scanner itself so that incompatible changes to ntdll.dll could be detected deterministically, instead of only after a multiple-match coincidence. 2. Call SetCurrentDirectory() and recheck the previously-matching addresses to see which one matches the new value. Place some limit (like 4) on the number of such retries, in case some new version of ntdll.dll intentionally duplicates the pointer every time. (Not sure what to do in that case; fall back to code scanning?) Sounds like code scanning is the better choice then. Your code scanner isn't overly complicated anyway, and it has the advantage of being rather fast. 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 14 10:12, Corinna Vinschen wrote:
On Sep 13 19:47, John Carey wrote:
Anyway, it looks to me as if overwriting the path in an existing
VistaCwd instance would confuse RtlGetCurrentDirectory_U() and
probably other Win32 API functions. (It may be that the same is
true for the handle member; I have not tried to find that out.)
Cygwin should probably allocate a new VistaCwd instance,
as does the Win32 SetCurrentDirectory() implementation.
Hmm, I'm still wondering. In theory we don't have to replace the
directory name. We just have to InterlockedExchange the handle, since
we only need to replace the original handle which does not allow
sharing-for-delete with our own handle which does.
All other border cases (virtual dir, directory with restricted rights)
can be rightfully handled by setting the Win32 CWD to \\?\PIPE\ again.
That's at least worth a try, isn't it?
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.
Corinna
Index: cygheap.h
===
RCS file: /cvs/src/src/winsup/cygwin/cygheap.h,v
retrieving revision 1.146
diff -u -p -r1.146 cygheap.h
--- cygheap.h 13 Aug 2010 11:51:53 - 1.146
+++ cygheap.h 14 Sep 2010 16:10:45 -
@@ -217,6 +217,8 @@ private:
a native Win32 application. See cwdstuff::set for
how it gets set. See spawn_guts for how it's
evaluated. */
+ void override_win32_cwd ();
+
public:
UNICODE_STRING win32;
static muto cwd_lock;
Index: path.cc
===
RCS file: /cvs/src/src/winsup/cygwin/path.cc,v
retrieving revision 1.608
diff -u -p -r1.608 path.cc
--- path.cc 14 Sep 2010 14:10:39 - 1.608
+++ path.cc 14 Sep 2010 16:10:45 -
@@ -3363,6 +3363,29 @@ get_user_proc_parms ()
return NtCurrentTeb ()-Peb-ProcessParameters;
}
+struct VistaCwd {
+ volatile LONG ReferenceCount;
+ HANDLE DirectoryHandle;
+ ULONG OldDismountCount;
+ UNICODE_STRING Path;
+ wchar_t Buffer[MAX_PATH];
+};
+
+void
+cwdstuff::override_win32_cwd ()
+{
+ if (wincap.has_transactions ())
+{
+ VistaCwd *v_cwd = (VistaCwd *)
+ ((PBYTE) get_user_proc_parms ()-CurrentDirectoryName.Buffer
+ - __builtin_offsetof (VistaCwd, Buffer));
+ (void) InterlockedExchangePointer (v_cwd-DirectoryHandle, dir);
+}
+ HANDLE h = InterlockedExchangePointer
+ (get_user_proc_parms ()-CurrentDirectoryHandle, dir);
+ NtClose (h);
+}
+
/* Initialize cygcwd 'muto' for serializing access to cwd info. */
void
cwdstuff::init ()
@@ -3370,9 +3393,13 @@ cwdstuff::init ()
cwd_lock.init (cwd_lock);
/* Cygwin processes inherit the cwd from their parent. If the win32 path
- buffer is not NULL, the cwd struct is already set up. */
- if (win32.Buffer)
-return;
+ buffer is not NULL, the cwd struct is already set up, and we only
+ have to override the Win32 CWD with ours. */
+ if (win32.Buffer !error)
+{
+ override_win32_cwd ();
+ return;
+}
/* Initially re-open the cwd to allow POSIX semantics. */
set (NULL, NULL);
@@ -3570,13 +3597,12 @@ cwdstuff::set (path_conv *nat_cwd, const
}
/* Keep the Win32 CWD in sync. Don't check for error, other than for
strace output. Try to keep overhead low. */
- if (nat_cwd)
-{
- status = RtlSetCurrentDirectory_U (error ? ro_u_pipedir : win32);
- if (!NT_SUCCESS (status))
- debug_printf (RtlSetCurrentDirectory_U(%S) failed, %p,
- error ? ro_u_pipedir : win32, status);
-}
+ status = RtlSetCurrentDirectory_U (error ? ro_u_pipedir : win32);
+ if (!NT_SUCCESS (status))
+debug_printf (RtlSetCurrentDirectory_U(%S) failed, %p,
+ error ? ro_u_pipedir : win32, status);
+ else if (!error)
+override_win32_cwd ();
/* Eventually, create POSIX path if it's not set on entry. */
tmp_pathbuf tp;
--
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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 14 18:11, Corinna Vinschen wrote: On Sep 14 10:12, Corinna Vinschen wrote: On Sep 13 19:47, John Carey wrote: Anyway, it looks to me as if overwriting the path in an existing VistaCwd instance would confuse RtlGetCurrentDirectory_U() and probably other Win32 API functions. (It may be that the same is true for the handle member; I have not tried to find that out.) Cygwin should probably allocate a new VistaCwd instance, as does the Win32 SetCurrentDirectory() implementation. Hmm, I'm still wondering. In theory we don't have to replace the directory name. We just have to InterlockedExchange the handle, since we only need to replace the original handle which does not allow sharing-for-delete with our own handle which does. All other border cases (virtual dir, directory with restricted rights) can be rightfully handled by setting the Win32 CWD to \\?\PIPE\ again. That's at least worth a try, isn't it? I applied the below patch to Cygwin CVS and it appears to work nicely. Sorry, that wasn't what I was trying to say. Actually, I'm using this code just locally for now and put it up here for discussion and for curious testers. I intend to apply it only, if we can agree that it's sufficient. 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. 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 14 01:12, Corinna Vinschen wrote: On Sep 13 19:47, John Carey wrote: On Sep 11 03:30, Corinna Vinschen: Anyway, it looks to me as if overwriting the path in an existing VistaCwd instance would confuse RtlGetCurrentDirectory_U() and probably other Win32 API functions. (It may be that the same is true for the handle member; I have not tried to find that out.) Cygwin should probably allocate a new VistaCwd instance, as does the Win32 SetCurrentDirectory() implementation. Hmm, I'm still wondering. In theory we don't have to replace the directory name. We just have to InterlockedExchange the handle, since we only need to replace the original handle which does not allow sharing-for-delete with our own handle which does. All other border cases (virtual dir, directory with restricted rights) can be rightfully handled by setting the Win32 CWD to \\?\PIPE\ again. That's at least worth a try, isn't it? Oh, I had thought that you wanted it for long paths and directories with restricted rights, too. There's one other issue: the new directory will limit deletion very briefly, until replaced. I don't know if that will bother people as much as the current situation. -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 14 17:39, John Carey wrote: On Sep 14 01:12, Corinna Vinschen wrote: On Sep 13 19:47, John Carey wrote: On Sep 11 03:30, Corinna Vinschen: Anyway, it looks to me as if overwriting the path in an existing VistaCwd instance would confuse RtlGetCurrentDirectory_U() and probably other Win32 API functions. (It may be that the same is true for the handle member; I have not tried to find that out.) Cygwin should probably allocate a new VistaCwd instance, as does the Win32 SetCurrentDirectory() implementation. Hmm, I'm still wondering. In theory we don't have to replace the directory name. We just have to InterlockedExchange the handle, since we only need to replace the original handle which does not allow sharing-for-delete with our own handle which does. All other border cases (virtual dir, directory with restricted rights) can be rightfully handled by setting the Win32 CWD to \\?\PIPE\ again. That's at least worth a try, isn't it? Oh, I had thought that you wanted it for long paths and directories with restricted rights, too. That would be nice, but to me it looks like the Win32 API is just too restricted for that, so that it won't make much sense. The native API can't make any assumption that it would work in such a scenario. Most Win32 calls will still not work in a restricted rights CWD, because they neglect to set the FILE_OPEN_FOR_BACKUP_INTENT flag. And most Apps don't set the FILE_FLAG_BACKUP_SEMANTICS flag incalls to CreateFile. As for long paths, there's some chance that the internal calls creating an absolute path from a relative path fail or crash if the CWD is longer than MAX_PATH. What MSDN has to say about the maximum path length of relative paths(*) is not very promising, at least. Also, even if Vista++ allows to create VistaCwd entries with a longer path, this would probably break starting native child processes due to the MAX_PATH restriction for the CWD in calls to CreateProcess. There's one other issue: the new directory will limit deletion very briefly, until replaced. I don't know if that will bother people as much as the current situation. True. Implementing a full replacement for SetCurrentDirectory as in your PoC is still an option. However, I can't do that anymore since I'm tainted by reading your code. If you would contemplate to sign a copyright assignment(**), we could create a more thorough solution with code scanning and all that in the long run. In the meantime, I could apply my patch and we can try how well it works, hacked as it is. Corinna (*) http://msdn.microsoft.com/en-us/library/aa365247%28VS.85%29.aspx -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 14 20:59, Corinna Vinschen wrote: On Sep 14 17:39, John Carey wrote: On Sep 14 01:12, Corinna Vinschen wrote: Hmm, I'm still wondering. In theory we don't have to replace the directory name. We just have to InterlockedExchange the handle, since we only need to replace the original handle which does not allow sharing-for-delete with our own handle which does. All other border cases (virtual dir, directory with restricted rights) can be rightfully handled by setting the Win32 CWD to \\?\PIPE\ again. That's at least worth a try, isn't it? Oh, I had thought that you wanted it for long paths and directories with restricted rights, too. That would be nice, but to me it looks like the Win32 API is just too restricted for that, so that it won't make much sense. The native API can't make any assumption that it would work in such a scenario. Most Win32 calls will still not work in a restricted rights CWD, because they neglect to set the FILE_OPEN_FOR_BACKUP_INTENT flag. And most Apps don't set the FILE_FLAG_BACKUP_SEMANTICS flag incalls to CreateFile. As for long paths, there's some chance that the internal calls creating an absolute path from a relative path fail or crash if the CWD is longer than MAX_PATH. What MSDN has to say about the maximum path length of relative paths(*) is not very promising, at least. Also, even if Vista++ allows to create VistaCwd entries with a longer path, this would probably break starting native child processes due to the MAX_PATH restriction for the CWD in calls to CreateProcess. There's one other issue: the new directory will limit deletion very briefly, until replaced. I don't know if that will bother people as much as the current situation. True. Implementing a full replacement for SetCurrentDirectory as in your PoC is still an option. However, I can't do that anymore since I'm tainted by reading your code. If you would contemplate to sign a copyright assignment(**), we could create a more thorough solution with code scanning and all that in the long run. In the meantime, I could apply my patch and we can try how well it works, hacked as it is. Corinna (*) http://msdn.microsoft.com/en-us/library/aa365247%28VS.85%29.aspx Sorry, I forgot the URL: (**) http://cygwin.com/contrib.html, the Before you get started section. 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 14 12:02, Corinna Vinschen wrote: True. Implementing a full replacement for SetCurrentDirectory as in your PoC is still an option. However, I can't do that anymore since I'm tainted by reading your code. If you would contemplate to sign a copyright assignment(**), we could create a more thorough solution with code scanning and all that in the long run. In the meantime, I could apply my patch and we can try how well it works, hacked as it is. Sorry, I had not realized that I would cause a problem by supplying proof of concept code. I'll inquire... -- 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 11 03:30, Corinna Vinschen: On Sep 11 00:12, John Carey wrote: The proof-of-concept code follows (and is also attached). It includes an analysis of what is going on--to the extent that I know what is going on, of course. Please let me know what you think. First of all, I'm thoroughly impressed. This looks like a lot of detective work and I'm also impressed by the amount of time you apparently put into this. Thank you. I'm hopeful we can create something for Cygwin from this. I have just a few discussing points for now. If it's useful, great. (It's up to you, of course, to decide if the maintainance burden is justified.) The PEB does NOT seem to point to any VistaCwd instances. Instead, That puzzles me a bit... After creating the new VistaCwd instance; call it newCwd, the SetCurrentDirectory () implementation modifies the PEB and Cwd under a lock on CwdCS, as follows: Params.CurrentDirectoryHandle = newCwd.DirectoryHandle; Params.CurrentDirectoryName.Buffer = newCwd.Path.Buffer; ...because, at this point it *does* point to the newCWD, even if only indirectly. Let's name the VistaCwd structure Cwd points to curCwd for now. Since we have the address of curCwd.Path.Buffer in Params.CurrentDirectoryName.Buffer, we can infer the address of curCwd from here, right? It's start is just a constant offset from the Buffer address. Excellent point. Even though the PEB does not contain the definitive pointer to the current VistaCwd instance, one can deduce its value, as you say. (One must also use the fact that newCwd.Path.Buffer == newCwd.Buffer.) Given that, wouldn't it be potentially possible to override the content of curCwd? The problem is probably (just as in my old Cygwin code up to 1.7.5) to make sure that this is thread safe. Probably we would still need CwdCS. Unfortunately, it looks as if Win32 API functions guarantee to each other that they will not modify the path in a VistaCwd whose reference count is = 2. (Though it looks like they may update OldDismountCount under a CwdCS lock.) Consider RtlGetCurrentDirectory_U(). It shares a (non-exported) subroutine with other Win32 API functions that acquires a counted reference to the current VistaCwd instance under the protection of a lock on CwdCS. (This subroutine also does some kind of freshness check against DismountCount.) But that subroutine releases its lock before returning, and RtlGetCurrentDirectory_U() then uses the pathname in the returned VistaCwd instance *without* holding a lock. Specifically, it copies that pathname to a memory buffer passed in by its own caller. Pathname copies are not atomic. Thus, unless there is a bug in RtlGetCurrentDirectory_U(), it has some guarantee that other threads will not modify the pathname in its VistaCwd instance, though they are free to allocate a new VistaCwd instance and assign its address to the global Cwd pointer (so long as they lock CwdCS). Presumably the point of the Vista++ CWD mechanism is to reduce contention on the CWD lock. Threads acquire that lock for very short periods of time. They appear to avoid making system calls or even allocating memory while holding it. Also, the CWD lock is no longer the PEB lock, so CWD usage does not serialize with other PEB-related activities. Anyway, it looks to me as if overwriting the path in an existing VistaCwd instance would confuse RtlGetCurrentDirectory_U() and probably other Win32 API functions. (It may be that the same is true for the handle member; I have not tried to find that out.) Cygwin should probably allocate a new VistaCwd instance, as does the Win32 SetCurrentDirectory() implementation. cout showbase hex (size_t)CwdCS == critical section endl; cout showbase hex (size_t)Cwd == Vista++ CWD struct pointer endl; Is there any connection between these two addresses, like, say, they are side by side? Not that I have been able to find. The gap between Cwd and CwdCS is always positive, but is large and varies among OS versions: 32-bit Vista: 428 bytes 32-bit 2008: 548 bytes 64-bit: Vista, 2008, and Windows 7: 8412 bytes Can't we find Cwd by scanning the .data segment of ntdll.h for the address we inferred from the Params.CurrentDirectoryName.Buffer value? Nice; that might work. But there would need to be some kind of rule to pick a winner among multiple matching words, in case by coincidence some other non-pointer word just happens to have the same bit pattern. I can see two alternatives for the multiple-match case: 1. Code scanning, as suggested earlier. There would need to be a unit test of the code scanner itself so that incompatible changes to ntdll.dll could be detected deterministically, instead of only after a multiple-match coincidence. 2. Call SetCurrentDirectory() and recheck the previously-matching addresses to see which one matches the new value. Place some limit (like 4) on the
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 13 12:47, John Carey wrote: On Sep 11 03:30, Corinna Vinschen: Can't we find Cwd by scanning the .data segment of ntdll.h for the address we inferred from the Params.CurrentDirectoryName.Buffer value? Nice; that might work. But there would need to be some kind of rule to pick a winner among multiple matching words, in case by coincidence some other non-pointer word just happens to have the same bit pattern. I can see two alternatives for the multiple-match case: 1. Code scanning, as suggested earlier. There would need to be a unit test of the code scanner itself so that incompatible changes to ntdll.dll could be detected deterministically, instead of only after a multiple-match coincidence. 2. Call SetCurrentDirectory() and recheck the previously-matching addresses to see which one matches the new value. Place some limit (like 4) on the number of such retries, in case some new version of ntdll.dll intentionally duplicates the pointer every time. (Not sure what to do in that case; fall back to code scanning?) Sorry, I just realized that while Cygwin might do all this to find Cwd, it will not work for CwdCS. There must be plenty of critical sections floating around. Still, reducing the number of addresses that must be discovered by code scanning might still help future-proof things. -- 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 13, John Carey wrote: It's not one of those, or even the subroutine that those share. I don't know how they relate. ... (incorrect quotation omitted) My apologies for using the wrong quotation format. Sorry! -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 11 00:12, John Carey wrote: On Sep 04 02:26 Corinna Vinschen wrote: The problem only starts with Vista. I have no objections to use undocumented features, if they work. If there's any way to replace the cwd handle with our own *and* keep the Win32 API happy, I'll take it. I think I've found a way to open the directory handle for backup intent on Vista and later versions. Essentially, I emulate the new things that SetCurrentDirectory() is doing, but in order to get the necessary addresses, I have to do some very ugly hacks. The proof-of-concept code follows (and is also attached). It includes an analysis of what is going on--to the extent that I know what is going on, of course. Please let me know what you think. First of all, I'm thoroughly impressed. This looks like a lot of detective work and I'm also impressed by the amount of time you apparently put into this. I'm hopeful we can create something for Cygwin from this. I have just a few discussing points for now. The PEB does NOT seem to point to any VistaCwd instances. Instead, That puzzles me a bit... After creating the new VistaCwd instance; call it newCwd, the SetCurrentDirectory () implementation modifies the PEB and Cwd under a lock on CwdCS, as follows: Params.CurrentDirectoryHandle = newCwd.DirectoryHandle; Params.CurrentDirectoryName.Buffer = newCwd.Path.Buffer; ...because, at this point it *does* point to the newCWD, even if only indirectly. Let's name the VistaCwd structure Cwd points to curCwd for now. Since we have the address of curCwd.Path.Buffer in Params.CurrentDirectoryName.Buffer, we can infer the address of curCwd from here, right? It's start is just a constant offset from the Buffer address. Given that, wouldn't it be potentially possible to override the content of curCwd? The problem is probably (just as in my old Cygwin code up to 1.7.5) to make sure that this is thread safe. Probably we would still need CwdCS. cout showbase hex (size_t)CwdCS == critical section endl; cout showbase hex (size_t)Cwd == Vista++ CWD struct pointer endl; Is there any connection between these two addresses, like, say, they are side by side? Can't we find Cwd by scanning the .data segment of ntdll.h for the address we inferred from the Params.CurrentDirectoryName.Buffer value? Thanks, 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
Btw... On Sep 11 00:12, John Carey wrote: // The real SetCurrentDirectory () implementation calls // a non-exported function that appears to expand relative // paths to absolute paths and convert / to \. It might // also do other things. Isn't that just one of RtlDosPathNameToNtPathName_U or RtlDosPathNameToNtPathName_U_WithStatus? Both functions are exported, afaics. 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 04 02:26 Corinna Vinschen wrote: On Sep 3 16:18, John Carey wrote: On Sep 03 12:37 Corinna Vinschen wrote: On Sep 2 23:32, John Carey wrote: In Aug 17 10:15, Corinna Vinschen wrote: I just released 1.7.6-1. ... What changed since Cygwin 1.7.5: - Cygwin handles the current working directory entirely on its own. The Win32 current working directory is set to an invalid path to be out of the way. This affects calls to the Win32 file API (CreateFile, etc.). See http://cygwin.com/htdocs/cygwin-ug-net/using.html#pathnames-win32-api Thank you very much for the fix! I've been running tests against Cygwin 1.7.6, and then 1.7.7, and those sporadic, non-deterministic failures in CreatePipe did stop after the 1.7.6 upgrade, and are still gone in 1.7.7. I think it's been long enough to conclude that it is not just the non-determinism--it really is fixed, as expected. I understand that this issue opened a can of worms; thanks again for your efforts to overcome those difficulties. I still don't like the final workaround, which is, to set the Win32 CWD to the Cygwin CWD. It would be nice if we could revert that change to the pre-1.7.6 behaviour in a Vista-friendly way. If you ever find out how to make sure that the new handle in the PEB's user parameter block is used even on Vista and later, pray tell me. Thus far the only ideas I have come up with are somewhat shaky and go well beyond the documented Win32 API. (If only there was the equivalent of dup2(), but for Win32 handles!!!) ACK. Just how much undocumented behavior is tolerable, do you think? Up to XP/2K3, we can simply set the handle in the user parameter block and be done with it, just as in 1.7.5, but without the Vista workaround. The problem only starts with Vista. I have no objections to use undocumented features, if they work. If there's any way to replace the cwd handle with our own *and* keep the Win32 API happy, I'll take it. I think I've found a way to open the directory handle for backup intent on Vista and later versions. Essentially, I emulate the new things that SetCurrentDirectory() is doing, but in order to get the necessary addresses, I have to do some very ugly hacks. The proof-of-concept code follows (and is also attached). It includes an analysis of what is going on--to the extent that I know what is going on, of course. Please let me know what you think. - - - - - - BEGIN INCLUSION - - - - - - - // Compile with Cygwin G++, and include a '-I' argument that // specifies the winsup directory of the Cygwin source tree. // // Run with two arguments: // // 1. An absolute Windows path to a directory (can use forward slashes). // // 2. The relative name of a file within that directory. // // The purpose of this source code is to discuss Win32 CWD // issues and to compile into a proof-of-concept program. // Please read the interleaved comments. #include w32api/include/windows.h #include w32api/include/ntdef.h #include w32api/include/winnt.h #include cygwin/ntdll.h #include algorithm #include cstdlib #include cstring #include iostream #include locale #include string using namespace std; / Primary research was on Windows 7 x64, but there appears to be at least superficial similarity on Windows 2008 (x32 and x64) and Windows Vista (x32 and x64). Let Params be an alias for *NtCurrentTeb ()-Peb-ProcessParameters. Let HeapHandle be an alias for *(PVOID*)((char*)NtCurrentTeb ()-Peb + 0x18) (which is the second 32-bit word after ProcessParameters.) Let DismountCount be an alias for the user space mapping of KUSER_SHARED_DATA::DismountCount: namely, *(ULONG*)0x7ffe02dc. See: http://www.nirsoft.net/kernel_struct/vista/KUSER_SHARED_DATA.html In the implementation of SetCurrentDirectory (), Params.CurrentDirectoryName.MaximumLength is read but NOT modified. Its value determines the sizes of various buffers, including the new buffer that will hold the new current directory pathname. The constant value we have observed for Params.CurrentDirectoryName.MaximumLength is 520, which is sizeof(wchar_t [MAX_PATH]). In addition to the PEB, there is an allocated memory block describing the current directory. Its lifetime is controlled by thread-safe reference counting. Call its type VistaCwd; more details follow. The PEB does NOT seem to point to any VistaCwd instances. Instead, there is a global pointer outside of the PEB, which we call Cwd, and it is protected by a critical section, which we call CwdCS, which is also outside of the PEB. Apparently these globals are in the ntdll.dll data space, but their symbols are not exported. Later we will discuss how to compute their addresses. NOTE: The SetCurrentDirectory () implementation appears to update the Win32 CWD *without* locking the PEB as
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 3 16:18, John Carey wrote: On Sep 03 12:37 Corinna Vinschen wrote: On Sep 2 23:32, John Carey wrote: In Aug 17 10:15, Corinna Vinschen wrote: I just released 1.7.6-1. ... What changed since Cygwin 1.7.5: - Cygwin handles the current working directory entirely on its own. The Win32 current working directory is set to an invalid path to be out of the way. This affects calls to the Win32 file API (CreateFile, etc.). See http://cygwin.com/htdocs/cygwin-ug-net/using.html#pathnames-win32-api Thank you very much for the fix! I've been running tests against Cygwin 1.7.6, and then 1.7.7, and those sporadic, non-deterministic failures in CreatePipe did stop after the 1.7.6 upgrade, and are still gone in 1.7.7. I think it's been long enough to conclude that it is not just the non-determinism--it really is fixed, as expected. I understand that this issue opened a can of worms; thanks again for your efforts to overcome those difficulties. I still don't like the final workaround, which is, to set the Win32 CWD to the Cygwin CWD. It would be nice if we could revert that change to the pre-1.7.6 behaviour in a Vista-friendly way. If you ever find out how to make sure that the new handle in the PEB's user parameter block is used even on Vista and later, pray tell me. Thus far the only ideas I have come up with are somewhat shaky and go well beyond the documented Win32 API. (If only there was the equivalent of dup2(), but for Win32 handles!!!) ACK. Just how much undocumented behavior is tolerable, do you think? Up to XP/2K3, we can simply set the handle in the user parameter block and be done with it, just as in 1.7.5, but without the Vista workaround. The problem only starts with Vista. I have no objections to use undocumented features, if they work. If there's any way to replace the cwd handle with our own *and* keep the Win32 API happy, I'll take it. 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 3 17:36, Andy Koppe wrote: On 3 September 2010 08:37, Corinna Vinschen wrote: I still don't like the final workaround, which is, to set the Win32 CWD to the Cygwin CWD. It would be nice if we could revert that change to the pre-1.7.6 behaviour in a Vista-friendly way. If you ever find out how to make sure that the new handle in the PEB's user parameter block is used even on Vista and later, pray tell me. I haven't got the magic bullet of course, but since we do have a way to make Win32-using programs work with just a relink, how about this transition scheme: - Add the previously suggested CYGWIN option for syncing the Win32 working directory, with two settings: on and off. Default to on for the moment. That would allow people like Daniel Colascione to switch it off if they don't care about the resulting failures in cygutils and elsewhere. - Implement the -lsynccwd scheme. - Notify maintainers about the rebuild that's necessary if their packages use Win32 APIs. Rebuild cygutils, git, and tcltk as soon as possible. - In a few months, switch the default to not sync, and hence allow working directories to be deleted., and which point. Affected programs that haven't been rebuilt could still be made to work by switching the option back on. That might work in the long run, but if there's any chance to get this automatically, as in 1.7.5, but without the Vista handle problem, I'd prefer it over any other workaround. For the time being, we have the EBUSY, but we can at least take comfort that it's backed by POSIX. 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 2 23:32, John Carey wrote: On Aug 12 01:11, Corinna Vinschen wrote: On Aug 12 06:54, Andy Koppe wrote: On 11 August 2010 20:55, John Carey wrote: So is your idea that if SetCurrentDirectory() fails because of path length or permissions, then Cygwin would just accept the failure and keep an internal record the POSIX current working directory and use that for all Cygwin calls, not the Win32 notion of current directory? Yes. The question then becomes what to do about the Win32 working directory in that case. Actually, Cygwin accepts *any* directory it can open as CWD: - Directories which can only be opened under SE_BACKUP_NAME. - Directories with a length up to 32768 chars. - Virtual directories, which don't exist at all as filesystem-based paths, like /proc, /cygdrive, etc. In Aug 17 10:15, Corinna Vinschen wrote: I just released 1.7.6-1. ... What changed since Cygwin 1.7.5: - Cygwin handles the current working directory entirely on its own. The Win32 current working directory is set to an invalid path to be out of the way. This affects calls to the Win32 file API (CreateFile, etc.). See http://cygwin.com/htdocs/cygwin-ug-net/using.html#pathnames-win32-api Thank you very much for the fix! I've been running tests against Cygwin 1.7.6, and then 1.7.7, and those sporadic, non-deterministic failures in CreatePipe did stop after the 1.7.6 upgrade, and are still gone in 1.7.7. I think it's been long enough to conclude that it is not just the non-determinism--it really is fixed, as expected. I understand that this issue opened a can of worms; thanks again for your efforts to overcome those difficulties. I still don't like the final workaround, which is, to set the Win32 CWD to the Cygwin CWD. It would be nice if we could revert that change to the pre-1.7.6 behaviour in a Vista-friendly way. If you ever find out how to make sure that the new handle in the PEB's user parameter block is used even on Vista and later, pray tell me. 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Sep 03 12:37 Corinna Vinschen wrote: On Sep 2 23:32, John Carey wrote: In Aug 17 10:15, Corinna Vinschen wrote: I just released 1.7.6-1. ... What changed since Cygwin 1.7.5: - Cygwin handles the current working directory entirely on its own. The Win32 current working directory is set to an invalid path to be out of the way. This affects calls to the Win32 file API (CreateFile, etc.). See http://cygwin.com/htdocs/cygwin-ug-net/using.html#pathnames-win32-api Thank you very much for the fix! I've been running tests against Cygwin 1.7.6, and then 1.7.7, and those sporadic, non-deterministic failures in CreatePipe did stop after the 1.7.6 upgrade, and are still gone in 1.7.7. I think it's been long enough to conclude that it is not just the non-determinism--it really is fixed, as expected. I understand that this issue opened a can of worms; thanks again for your efforts to overcome those difficulties. I still don't like the final workaround, which is, to set the Win32 CWD to the Cygwin CWD. It would be nice if we could revert that change to the pre-1.7.6 behaviour in a Vista-friendly way. If you ever find out how to make sure that the new handle in the PEB's user parameter block is used even on Vista and later, pray tell me. Thus far the only ideas I have come up with are somewhat shaky and go well beyond the documented Win32 API. (If only there was the equivalent of dup2(), but for Win32 handles!!!) Just how much undocumented behavior is tolerable, do you think? -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On 3 September 2010 08:37, Corinna Vinschen wrote: I still don't like the final workaround, which is, to set the Win32 CWD to the Cygwin CWD. It would be nice if we could revert that change to the pre-1.7.6 behaviour in a Vista-friendly way. If you ever find out how to make sure that the new handle in the PEB's user parameter block is used even on Vista and later, pray tell me. I haven't got the magic bullet of course, but since we do have a way to make Win32-using programs work with just a relink, how about this transition scheme: - Add the previously suggested CYGWIN option for syncing the Win32 working directory, with two settings: on and off. Default to on for the moment. That would allow people like Daniel Colascione to switch it off if they don't care about the resulting failures in cygutils and elsewhere. - Implement the -lsynccwd scheme. - Notify maintainers about the rebuild that's necessary if their packages use Win32 APIs. Rebuild cygutils, git, and tcltk as soon as possible. - In a few months, switch the default to not sync, and hence allow working directories to be deleted., and which point. Affected programs that haven't been rebuilt could still be made to work by switching the option back on. Andy -- 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Aug 12 01:11, Corinna Vinschen wrote: On Aug 12 06:54, Andy Koppe wrote: On 11 August 2010 20:55, John Carey wrote: So is your idea that if SetCurrentDirectory() fails because of path length or permissions, then Cygwin would just accept the failure and keep an internal record the POSIX current working directory and use that for all Cygwin calls, not the Win32 notion of current directory? Yes. The question then becomes what to do about the Win32 working directory in that case. Actually, Cygwin accepts *any* directory it can open as CWD: - Directories which can only be opened under SE_BACKUP_NAME. - Directories with a length up to 32768 chars. - Virtual directories, which don't exist at all as filesystem-based paths, like /proc, /cygdrive, etc. In Aug 17 10:15, Corinna Vinschen wrote: I just released 1.7.6-1. ... What changed since Cygwin 1.7.5: - Cygwin handles the current working directory entirely on its own. The Win32 current working directory is set to an invalid path to be out of the way. This affects calls to the Win32 file API (CreateFile, etc.). See http://cygwin.com/htdocs/cygwin-ug-net/using.html#pathnames-win32-api Thank you very much for the fix! I've been running tests against Cygwin 1.7.6, and then 1.7.7, and those sporadic, non-deterministic failures in CreatePipe did stop after the 1.7.6 upgrade, and are still gone in 1.7.7. I think it's been long enough to conclude that it is not just the non-determinism--it really is fixed, as expected. I understand that this issue opened a can of worms; thanks again for your efforts to overcome those difficulties. -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Aug 12 06:54, Andy Koppe wrote: On 11 August 2010 20:55, John Carey wrote: On Aug 11 01:49 Corinna Vinschen wrote: there's no Win32-safe way to set a new directory handle as cwd in Vista and later anymore. Since there's no official API to set the cwd using a directory handle, there's no way to set the Win32 cwd to a directory with restricted permissions. This *is* frustrating. I'll look into another solution. Probably we will have to call SetCurrentDirectory again and ignore any error. I don't accept the aforementioned restriction for POSIX calls. So is your idea that if SetCurrentDirectory() fails because of path length or permissions, then Cygwin would just accept the failure and keep an internal record the POSIX current working directory and use that for all Cygwin calls, not the Win32 notion of current directory? Yes. The question then becomes what to do about the Win32 working directory in that case. Actually, Cygwin accepts *any* directory it can open as CWD: - Directories which can only be opened under SE_BACKUP_NAME. - Directories with a length up to 32768 chars. - Virtual directories, which don't exist at all as filesystem-based paths, like /proc, /cygdrive, etc. 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Aug 10 21:53, John Carey wrote: On Aug 10 13:44 +0200 Corinna Vinschen wrote: chdir (/); h = CreateFile (foobar, ...); if (h == INVALID_HANDLE_VALUE) printf (%lu\n, GetLastError()); Thanks for the test case for the CreateFile() problem; I used it to create the following test, in which Windows 7 CreateFile() fails with ERROR_INVALID_HANDLE even when using a stock Cygwin 1.7.5 DLL: [...] Thanks for the test case. The PEB lock in cwdstuff::set() can only prevent the race if other threads always hold the PEB lock while invoking system calls that allocate handles. CreateFile() is pretty big, so I might have missed it, but I did not see CreateFile() holding the PEB lock while actually creating the new file handle. No, it doesn't have to. It only needs the PEB lock when accessing the cwd in the PEB. But ... Instead, after actually creating the new file handle, it called _rtlreleaserelativen...@4 on a reference-counted object that holds a copy of the current directory handle (see below for more about such reference-counted current directory objects). ... if that's correct it seems that the cwd from the PEB isn't even used in Vista++. That explains why just exchanging the handle value in cwdstuff::set() only works fine in 2K3 and earlier systems. Bummer. I assume the idea was to speed up CreateFile by getting rid of a lock per call. Now, where is Windows stuffing the extra copy of the directory handle? In those reference-counted heap-allocated directory objects. Stepping through the machine code under Windows 7 I see SetCurrentDirectory() updating a global pointer (ntdll!RtlpCurDirRef) to one such object, under the protection of a critical section (ntdll!FastPebLock). Despite mentioning Peb in the name, neither global's address is computed using FS:. The global pointer points to a heap-allocated block that starts with a reference count followed by a copy of the directory handle value, and apparently includes other data as well. SetCurrentDirectory() swaps in a new such block, and decrements the counter on the old block, and if that reaches 0, closes the handle. Anyway, this block appears to be where the old current directory handle value persists past the changes made by cwdstuff::set(). ...and that handle is used in subsequent calls and potentially is a handle to another object in the meantime. I do basically agree with cgf that it's your own problem if you use Win32 calls in your Cygwin app. OTOH, I see that this can trip up potential handle problems in the DLL itself. Unfortunately that means there's no Win32-safe way to set a new directory handle as cwd in Vista and later anymore. Since there's no official API to set the cwd using a directory handle, there's no way to set the Win32 cwd to a directory with restricted permissions. This *is* frustrating. I'll look into another solution. Probably we will have to call SetCurrentDirectory again and ignore any error. I don't accept the aforementioned restriction for POSIX calls. 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
Greetings, Al Slater! -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/08/2010 09:57, Andrey Repin wrote: Greetings, John Carey! After a call to SetCurrentDirectory(), I have seen occasional ( 5%) failures of CreatePipe() with code ERROR_INVALID_HANDLE. I have also seen failure of Cygwin signal handling because the signal handling pipe has mysteriously closed. Seems like it was discussed a short while ago. mid:008101cb3597$a75069f0$f5f13d...@gmail.com Out of interest, what is that strange email address above supposed to refer to? mid - message ID. If you have normal mail/news reader, and you have this message in your local database, it will be a link and clicking it will bring you to that message. -- WBR, Andrey Repin (anrdae...@freemail.ru) 11.08.2010, 13:29 Sorry for my terrible english... -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On 8/10/10 10:44 PM, Christopher Faylor wrote: On Tue, Aug 10, 2010 at 09:53:46PM +, John Carey wrote: Thanks for the test case for the CreateFile() problem; I used it to create the following test, in which Windows 7 CreateFile() fails with ERROR_INVALID_HANDLE even when using a stock Cygwin 1.7.5 DLL: As Corinna said: If you are mixing Windows calls with cygwin calls you are far into unsupported territory. Cygwin isn't designed to let you seamlessly intermix things like pthreads/signals and the Windows API functions. Then it seems the website needs to be updated. One of the major selling points of Cygwin, as it were, is the ability to interact with the Windows API world as well as the POSIX one: Because processes run under the standard Win32 subsystem, they can access both the UNIX compatibility calls provided by Cygwin as well as any of the Win32 API calls. This gives the programmer complete flexibility in designing the structure of their program in terms of the APIs used. For example, they could write a Win32-specific GUI using Win32 API calls on top of a UNIX back-end that uses Cygwin. I can understand requiring that users not deliberately create inconsistencies between Cygwin data structures and those of Windows (and I can understand throwing people who do that to the wolves), but CreateFile is a fundamental system facility that should always work, even in a Cygwin process. -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Aug 11 04:05, Daniel Colascione wrote: On 8/10/10 10:44 PM, Christopher Faylor wrote: On Tue, Aug 10, 2010 at 09:53:46PM +, John Carey wrote: Thanks for the test case for the CreateFile() problem; I used it to create the following test, in which Windows 7 CreateFile() fails with ERROR_INVALID_HANDLE even when using a stock Cygwin 1.7.5 DLL: As Corinna said: If you are mixing Windows calls with cygwin calls you are far into unsupported territory. Cygwin isn't designed to let you seamlessly intermix things like pthreads/signals and the Windows API functions. Then it seems the website needs to be updated. One of the major selling points of Cygwin, as it were, is the ability to interact with the Windows API world as well as the POSIX one: Because processes run under the standard Win32 subsystem, they can access both the UNIX compatibility calls provided by Cygwin as well as any of the Win32 API calls. This gives the programmer complete flexibility in designing the structure of their program in terms of the APIs used. For example, they could write a Win32-specific GUI using Win32 API calls on top of a UNIX back-end that uses Cygwin. I can understand requiring that users not deliberately create inconsistencies between Cygwin data structures and those of Windows (and I can understand throwing people who do that to the wolves), but CreateFile is a fundamental system facility that should always work, even in a Cygwin process. There's a problem, and the current problem is a typical representation for this, so let's stick to this example. In Cygwin it is possible to chdir to a directory to which you can't cd using the SetCurrentDirectory call. If you're an administrator, you have BACKUP_NAME privileges. So you can access files in directories which don't give you this permission usually, just like a root user on Linux. Unfortunately the SetCurrentDirectory call doesn't support this. It only takes a Win32 pathname which is subsequently opened without providing the FILE_FLAG_BACKUP_SEMANTICS flag. So, while you can *open* files in such a dir using the FILE_FLAG_BACKUP_SEMANTICS flag, you can't set your CWD to it when using the Win32 API. Another exmaple: Windows supports path of up to 32768 bytes. However, the Win32 CWD can have a maximum length of 259 chars. Even when using the UNICODE API! None of these restriction exist for Cygwin applications *iff* they use the POSIX API. So, what if a Cygwin application chdir'ed into such a directory successfully? If I change back Cygwin to use SetCurrentDirectory to keep the Win32 API in sync, the call will fail in the first example, while the second example is never supported in the Win32 API. In effect, the POSIX API CWD is now a different one than the Win32 API CWD. So, what does sticking to Win32 compatibility mean? Does that mean we have still the right to make the POSIX API as good as possible, thus risking that CreateFile with a relative filename fails or accesses another file than the POSIX open call? Or does that mean we have to revert all these changes since we don't have the right to provide better or more functionality in the POSIX API if it potentially breaks the Win32 API? 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Wed, Aug 11, 2010 at 04:05:32AM -0700, Daniel Colascione wrote: On 8/10/10 10:44 PM, Christopher Faylor wrote: On Tue, Aug 10, 2010 at 09:53:46PM +, John Carey wrote: Thanks for the test case for the CreateFile() problem; I used it to create the following test, in which Windows 7 CreateFile() fails with ERROR_INVALID_HANDLE even when using a stock Cygwin 1.7.5 DLL: As Corinna said: If you are mixing Windows calls with cygwin calls you are far into unsupported territory. Cygwin isn't designed to let you seamlessly intermix things like pthreads/signals and the Windows API functions. Then it seems the website needs to be updated. One of the major selling points of Cygwin, as it were, is the ability to interact with the Windows API world as well as the POSIX one: Because processes run under the standard Win32 subsystem, they can access both the UNIX compatibility calls provided by Cygwin as well as any of the Win32 API calls. This gives the programmer complete flexibility in designing the structure of their program in terms of the APIs used. For example, they could write a Win32-specific GUI using Win32 API calls on top of a UNIX back-end that uses Cygwin. I've removed that paragraph. Thanks for the heads up. cgf -- 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
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Aug 11 01:49 Corinna Vinschen wrote: On Aug 10 21:53, John Carey wrote: Now, where is Windows stuffing the extra copy of the directory handle? In those reference-counted heap-allocated directory objects. Stepping through the machine code under Windows 7 I see SetCurrentDirectory() updating a global pointer (ntdll!RtlpCurDirRef) to one such object, under the protection of a critical section (ntdll!FastPebLock). Despite mentioning Peb in the name, neither global's address is computed using FS:. The global pointer points to a heap-allocated block that starts with a reference count followed by a copy of the directory handle value, and apparently includes other data as well. SetCurrentDirectory() swaps in a new such block, and decrements the counter on the old block, and if that reaches 0, closes the handle. Anyway, this block appears to be where the old current directory handle value persists past the changes made by cwdstuff::set(). ...and that handle is used in subsequent calls and potentially is a handle to another object in the meantime. I do basically agree with cgf that it's your own problem if you use Win32 calls in your Cygwin app. OTOH, I see that this can trip up potential handle problems in the DLL itself. Unfortunately that means there's no Win32-safe way to set a new directory handle as cwd in Vista and later anymore. Since there's no official API to set the cwd using a directory handle, there's no way to set the Win32 cwd to a directory with restricted permissions. This *is* frustrating. I'll look into another solution. Probably we will have to call SetCurrentDirectory again and ignore any error. I don't accept the aforementioned restriction for POSIX calls. So is your idea that if SetCurrentDirectory() fails because of path length or permissions, then Cygwin would just accept the failure and keep an internal record the POSIX current working directory and use that for all Cygwin calls, not the Win32 notion of current directory? -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On 11 August 2010 20:55, John Carey wrote: On Aug 11 01:49 Corinna Vinschen wrote: there's no Win32-safe way to set a new directory handle as cwd in Vista and later anymore. Since there's no official API to set the cwd using a directory handle, there's no way to set the Win32 cwd to a directory with restricted permissions. This *is* frustrating. I'll look into another solution. Probably we will have to call SetCurrentDirectory again and ignore any error. I don't accept the aforementioned restriction for POSIX calls. So is your idea that if SetCurrentDirectory() fails because of path length or permissions, then Cygwin would just accept the failure and keep an internal record the POSIX current working directory and use that for all Cygwin calls, not the Win32 notion of current directory? Yes. The question then becomes what to do about the Win32 working directory in that case. Andy -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
Greetings, John Carey! After a call to SetCurrentDirectory(), I have seen occasional ( 5%) failures of CreatePipe() with code ERROR_INVALID_HANDLE. I have also seen failure of Cygwin signal handling because the signal handling pipe has mysteriously closed. Seems like it was discussed a short while ago. mid:008101cb3597$a75069f0$f5f13d...@gmail.com -- WBR, Andrey Repin (anrdae...@freemail.ru) 10.08.2010, 12:56 Sorry for my terrible english... -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Aug 10 00:19, John Carey wrote: After a call to SetCurrentDirectory(), I have seen occasional ( 5%) failures of CreatePipe() with code ERROR_INVALID_HANDLE. I have also seen failure of Cygwin signal handling because the signal handling pipe has mysteriously closed. Why on earth are you calling SetCurrentDirectory in a Cygwin application? 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 undocumented 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 (), phdl, 0, TRUE, DUPLICATE_SAME_ACCESS)) NtClose (h); you missed the else *phdl = 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. Note that phdl is a pointer to the handle storage within the PEB. Since the PEB is locked (RtlAcquirePebLock/RtlReleasePebLock) and Cygwin's cwdstuff::set as well as the native SetCurrentDirectory both lock the PEB before writing the CurDir content, there's no chance for a concurrency issue between those functions. Also note that the old content of *phdl is *always* overwritten, either by the result of the DuplicateHandle call, or by the value of CreateFile. Presumably the issues mentioned in the source comment may also occur, but what I have seen myself is a subsequent call to SetCurrentDirectory() closing, not the current directory handle, but the handle that was allocated by the other thread. Specifically, dll_crt0_1() calls sigproc_init(), then cwdstuff::set(), and finally wait_for_sigthread(). The function sigproc_init() creates the signal handling thread, which creates the signal handling pipe as one of its very first actions, then sets the event for which wait_for_sigthread() waits. I think this scenario happens: 1. main thread: cwdstuff::set(): NtClose(). 2. signal handling thread: CreatePipe() opens a handle to '\Device\NamedPipe\' 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. No, that's not intended. However, the code just *tries* to preserve the handle value, but it does not rely on it. The NtClose is safe because the handle is the actual CWD handle at the time of the call and the PEB is locked. The DuplicateHandle call just uses the phdl storage to store the new handle value, but it *never* tests if the new handle value is identical to the old one. So, if all works well, it's the same handle value as before. If not, *shrug*. CreateFile to fail with ERROR_INVALID_HANDLE as mentioned in the source comments, but I have not seen that. I think that the I have. You can easily test this as well on Vista and W7. Just drop the DuplicateHandle call and simply store h in *phdl. Then create a testcase: chdir (/); h = CreateFile (foobar, ...); if (h == INVALID_HANDLE_VALUE) printf (%lu\n, GetLastError()); CreatePipe() failures I have seen are triggered when SetCurrentDirectory() closes the handle to '\Device\NamedPipe\', thinking that it is the current directory handle. After that, CreatePipe() will fail with ERROR_INVALID_HANDLE. As mentioned above, calling SetCurrentDirectory in a Cygwin application is not correct at all. All relative path handling in Cygwin relies on the fact that a Cygwin application calls the chdir function. If you're calling SetCurrentDirectory you're on your own. And then again, note that the call to SetCurrentDirectory will not overwrite the PEB value of the cwd handle until the cwdstuff::set function has called RtlReleasePebLock. I think this other scenario also happens: 1. signal handling thread: CreatePipe() opens a handle to '\Device\NamedPipe\' (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 calls to SetCurrentDirectory(), because they close one of the pipe handles used for Cygwin signal handling. This is pure
RE: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
Greetings to you as well! I did try to search, but was unsuccessful. Can you tell me the subject of one of the messages? Thanks, John Subject: Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set Greetings, John Carey! After a call to SetCurrentDirectory(), I have seen occasional ( 5%) failures of CreatePipe() with code ERROR_INVALID_HANDLE. I have also seen failure of Cygwin signal handling because the signal handling pipe has mysteriously closed. Seems like it was discussed a short while ago. mid:008101cb3597$a75069f0$f5f13d...@gmail.com -- WBR, Andrey Repin (anrdae...@freemail.ru) 10.08.2010, 12:56 Sorry for my terrible english... -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/08/2010 09:57, Andrey Repin wrote: Greetings, John Carey! After a call to SetCurrentDirectory(), I have seen occasional ( 5%) failures of CreatePipe() with code ERROR_INVALID_HANDLE. I have also seen failure of Cygwin signal handling because the signal handling pipe has mysteriously closed. Seems like it was discussed a short while ago. mid:008101cb3597$a75069f0$f5f13d...@gmail.com Out of interest, what is that strange email address above supposed to refer to? - -- Al -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (SunOS) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkxhqb8ACgkQz4fTOFL/EDYW1gCePOArKa9PB8qhvNwaeQgtGfTt v6cAn1ixv/R9+BvWnD7vSgxY2sSkj9t6 =DTsS -END PGP SIGNATURE- -- 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
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Aug 10 13:44 +0200 Corinna Vinschen wrote:
On Aug 10 00:19, John Carey wrote:
...
Presumably the issues mentioned in the source comment may
also occur, but what I have seen myself is a subsequent call to
SetCurrentDirectory() closing, not the current directory handle, but
the handle that was allocated by the other thread.
Specifically, dll_crt0_1() calls sigproc_init(), then cwdstuff::set(),
and finally wait_for_sigthread(). The function sigproc_init() creates
the signal handling thread, which creates the signal handling pipe as
one of its very first actions, then sets the event for which
wait_for_sigthread() waits. I think this scenario happens:
1. main thread: cwdstuff::set(): NtClose().
2. signal handling thread: CreatePipe() opens a handle to
'\Device\NamedPipe\' 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.
No, that's not intended. However, the code just *tries* to preserve the
handle value, but it does not rely on it. The NtClose is safe because
the handle is the actual CWD handle at the time of the call and the PEB
is locked. The DuplicateHandle call just uses the phdl storage to store
the new handle value, but it *never* tests if the new handle value is
identical to the old one. So, if all works well, it's the same handle
value as before. If not, *shrug*.
CreateFile to fail with ERROR_INVALID_HANDLE as mentioned in the
source comments, but I have not seen that. I think that the
I have. You can easily test this as well on Vista and W7. Just drop
the DuplicateHandle call and simply store h in *phdl. Then create a
testcase:
chdir (/);
h = CreateFile (foobar, ...);
if (h == INVALID_HANDLE_VALUE) printf (%lu\n, GetLastError());
Thanks for the test case for the CreateFile() problem; I used it to
create the following test, in which Windows 7 CreateFile() fails with
ERROR_INVALID_HANDLE even when using a stock Cygwin 1.7.5 DLL:
bu...@aeolus-w764c17 ~
$ uname -a
CYGWIN_NT-6.1-WOW64 aeolus-w764c17 1.7.5(0.225/5/3) 2010-04-12 19:07 i686
Cygwin
bu...@aeolus-w764c17 ~
$ cat test.c
#include windows.h
#include pthread.h
#include unistd.h
#include stdio.h
#include stdlib.h
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
void * run (void *arg)
{
int count = (int)arg 1;
int sync = (int)arg 1;
while (count--) {
if (sync) pthread_mutex_lock(mutex);
HANDLE h = CreateFile (
foobar,
GENERIC_WRITE,
0,
NULL,
CREATE_ALWAYS,
FILE_ATTRIBUTE_NORMAL,
INVALID_HANDLE_VALUE);
if (h == INVALID_HANDLE_VALUE) {
printf (%lu\n, GetLastError());
exit(1);
}
CloseHandle(h);
if (sync) pthread_mutex_unlock(mutex);
}
return 0;
}
int main (int argc, char** argv)
{
int count;
int sync;
pthread_t tid;
void *result;
if (argc != 2 argc != 3) {
fprintf(stderr, Usage: %s count [sync]\n, argv[0]);
return 2;
}
count = atol (argv[1]);
sync = argc = 3 ? !!atoi(argv[2]) : 0;
pthread_create (tid, 0, run, (void*)((count 1) | sync));
while (count--) {
if (sync) pthread_mutex_lock(mutex);
chdir (/);
if (sync) pthread_mutex_unlock(mutex);
}
pthread_join (tid, result);
return 0;
}
bu...@aeolus-w764c17 ~
$ gcc -o ~/test ~/test.c
bu...@aeolus-w764c17 ~
$ ~/test 1000 1
123
bu...@aeolus-w764c17 ~
$ ~/test 1000 0
123
bu...@aeolus-w764c17 ~
$ cd /
bu...@aeolus-w764c17 /
$ ~/test 1000 1
bu...@aeolus-w764c17 /
$ ~/test 1000 0
6
bu...@aeolus-w764c17 /
$
6 = The handle is invalid.:
I think that this is the error previously discussed.
Note that passing 1 as the second program argument
synchronizes the two threads and prevents the error.
123 = The filename, directory name, or volume label syntax is incorrect.:
I have not yet investigated why that error occurs, but it does
not happen if I run the test in the Cygwin root directory.
...
I think this other scenario also happens:
1. signal handling thread: CreatePipe() opens a handle to
'\Device\NamedPipe\' (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 calls to SetCurrentDirectory(), because they close one of
the pipe handles used for Cygwin signal handling.
This is pure speculation. Please explain to me how DuplicateHandle,
which duplicates a *local*
Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set
On Tue, Aug 10, 2010 at 09:53:46PM +, John Carey wrote: Thanks for the test case for the CreateFile() problem; I used it to create the following test, in which Windows 7 CreateFile() fails with ERROR_INVALID_HANDLE even when using a stock Cygwin 1.7.5 DLL: As Corinna said: If you are mixing Windows calls with cygwin calls you are far into unsupported territory. Cygwin isn't designed to let you seamlessly intermix things like pthreads/signals and the Windows API functions. If you can demonstrate a problem with pure Cygwin calls then go ahead and post a test case. Otherwise, I don't see this as an issue that needs to be addressed. cgf -- 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
