Menu
▾
▴
[MinGW-notify] [ mingw-Bugs-3302830 ] Terminates silently if /etc is missing
|
From: SF/projects/mingw n. l. <min...@li...> - 2011-05-20 08:03:42
|
Bugs item #3302830, was opened at 2011-05-16 14:05 Message generated for change (Comment added) made by fabian_deb You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=3302830&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Fabian Greffrath (fabian_deb) Assigned to: Cesar Strauss (cstrauss) Summary: Terminates silently if /etc is missing Initial Comment: Hi, currently, if one tries to create a "portable" MSYS by copying all the .exe and .dll files from the c:\mingw\msys\1.0\bin directory to e.g. a flash drive, the MSYS programs may terminate silently without any output and error messages. The reason is that the ../etc directory (relative to msys-1.0.dll) is missing. There is a thread in msys-1.0.dll called read_mounts_thread that watches for changes in /etc and calls ExitProcess (1) if the directory does not exist (or is unreadable or whatever INVALID_HANDLE_VALUE may mean). However, I don't see why the missing /etc directory should cause the whole library to exit! I mean, nothing inside the /etc directory is strictly required as creating an entire empty one will fix the issue. IMHO, as long as /etc/fstab is missing anyway, a missing /etc directory and an empty one should be handled the same by the library. I therefore suggest to replace the call to "ExitProcess (1)" in line 1668 of file source\winsup\cygwin\path.cc in the msysCORE sources by "return (0)" and thus simply closing the thread instead of the whole process if the /etc directory is not readable. Thanks for considering! - Fabian ---------------------------------------------------------------------- >Comment By: Fabian Greffrath (fabian_deb) Date: 2011-05-20 10:03 Message: > Or to simply mkdir() the missing /etc > directory, and add an empty /etc/fstab file? I think creating only the /etc directory should be sufficient, the thread just needs something that it can actually watch. The code would become rather ugly, however, since we would have to run the same FindFirstChangeNotification call again and check its return value after the mkdir(). So earnie, do you suggest to apply my original patch and just return from the thread when /etc is not watchable? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2011-05-20 00:40 Message: > Will it consume any more CPU cycles than the existing loop on > FindNextChangeNotification()? The existing loop has a blocking call to WaitForSingleObject. It should consume cycles only when the directory contents actually changes. > If looping on FindFirstChangeNotification() is unacceptable, would it be > possible to make it block? Indeed. I suggest using FindFirstChangeNotification / FindNextChangeNotification / WaitForSingleObject on the PARENT directory, waiting for the creation of the etc directory itself. Cesar ---------------------------------------------------------------------- Comment By: Earnie Boyd (earnie) Date: 2011-05-20 00:07 Message: Your original patch, the thread will only remain closed for the initial session. Once /etc is created and the session exited and restarted the thread will remain open. Silently creating /etc could have undesirable effects as well, such as not being able to create the directory. Is it worth the overhead to check it in the thread, no, the check for /etc existing should be in the dll initialization code so that it only happens once during dll startup. ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2011-05-19 10:39 Message: >> Put FindFirstChangeNotification into a loop and repeat >> it until ../etc is finally accessible, then proceed to the loop around >> FindNextChangeNotification as usual. > > Won't this loop consume CPU cycles? Will it consume any more CPU cycles than the existing loop on FindNextChangeNotification()? > I'd rather use your original suggestion. I concur with Fabian's conclusion that the loss of (mount table) functionality which that incurs would be undesirable -- although certainly better than the current (silently abort) behaviour. If looping on FindFirstChangeNotification() is unacceptable, would it be possible to make it block? Or to simply mkdir() the missing /etc directory, and add an empty /etc/fstab file? (Perhaps with a warning diagnostic to this effect)? ---------------------------------------------------------------------- Comment By: Fabian Greffrath (fabian_deb) Date: 2011-05-19 09:21 Message: > Won't this loop consume CPU cycles? I'd rather use your original > suggestion. The original loop around FindNextChangeNotification does consume CPU cycles as well. The main advantage against my first suggestion is that you can add an /etc/fstab file, even if it was initially missing when the process was started, and then newly added mounts are still handled as expected. This is impossible with my first suggestion, which closes the thread for the whole runtime of the process if there is no /etc/fstab at start. - Fabian ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2011-05-19 00:17 Message: > Put FindFirstChangeNotification into a loop and repeat > it until ../etc is finally accessible, then proceed to the loop around > FindNextChangeNotification as usual. Won't this loop consume CPU cycles? I'd rather use your original suggestion. Regards, Cesar ---------------------------------------------------------------------- Comment By: Fabian Greffrath (fabian_deb) Date: 2011-05-18 13:35 Message: Just in case you didn't like the naive approach that my first patch took, here is another one: Put FindFirstChangeNotification into a loop and repeat it until ../etc is finally accessible, then proceed to the loop around FindNextChangeNotification as usual. I have tested this patch and it actually works as expected! I can start bash from a directory that contains nothing more than bash.exe itself and the needed shared libraries, i.e. my patched msys-1.0.dll and stock msys-iconv-2.dll, msys-intl-8.dll and msys-regex-1.dll. There is no etc directory in the directory above. With the stock msys-1.0.dll, bash just quits silently, but with my patch it actually runs. If I also copy over mount and msysmnt.exe, I can run mount to monitor the mount points. As long as there is no ../etc directory, mount will complain about a missing /etc/fstab and only show the usual mount points, e.g. c: -> /c. As soon as I create the etc directory in the directory above and add an entry to its fstab file, this new mount point is instantly available in bash as well (i.e. the loop around FindNextChangeNotification has successfully been entered). If I blank out the ../etc/fstab file again, the just added mount point instantly disappears. Removing the ../etc/fstab file or the ../etc directory is not possible ("access denied"), though, but I guess that is expected due to the watching thread and not a side effect of my patch. I'd be glad to provide more information or examples if needed! Cheers, Fabian ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=3302830&group_id=2435 |