On Aug 6, 2010, at 5:42 PM, Jan Dubois wrote:
> Against my better judgment I couldn't prevent myself from peeking at
> the magical patch. I don't understand what it is doing, and I don't
> have any candies so sacrifice right now either.
>
> However, I'm always suspicious when I see code like this:
>
> if ($^O =~ /win/i) { #Win32 dark magic. It works, so don't change anything
>
> It isn't clear if that branch should be executed for Cygwin or not (currently
> it will, although the comment sounds like it shouldn't, given that Cygwin is
> considered to be a separate platform from Win32). I prefer to always make
> things
> explicit, either
>
> if ($^O eq "MSWin32" || $^O eq "cygwin") {
>
> or
>
> if ($^O eq "MSWin32") {Agreed. I suspect that cygwin uses actual forking, no? > Of course the "It works, so don't change anything" comment is another alarming > red flag: you should not make changes to code if you don't understand what the > changes are doing and/or if you can't explain why it does what it does. Just > because you are getting rid of a symptom doesn't mean you solved the real > issue. I *think* it works the way it does because on Win32 it's not really a fork, but a thread, and when you exit in a child thread, it exits the parent, too. Correct? If so, then the attached patch should work just as well. Alexandr, can you give it a try? Oh, and BTW, you're my hero for looking into this! Best, David
win32-destroy.patch
Description: Binary data
