Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
Please do not cull the Cc list. Am 3/28/2014 11:07, schrieb Marat Radchenko: > Jeff King peff.net> writes: > >> >> I'm not sure what an actual SIGPIPE death looks like on Windows. > > There is no SIGPIPE death on Windows due to total absence of SIGPIPE. > raise(unsupported int) just causes ugly "git.exe has stopped working" > window and possibly ends up as SIGABT (I don't know how to check this). This happens "only" with newer Microsoft C runtime libraries. They do not return EINVAL (because that usually indicates a bug caused by insufficient checks before the function call), but crash the program by default in the way that you observed. >> What >> happens if git is still writing data to the pager and the pager exits? >> Does it receive a signal of some sort? No; the write attempt returns with EPIPE. > > I'm not sure what you mean, sorry. check_pipe properly detects pager exit. > The problem is with the way it tries to die. > >> The point of the code in check_pipe is to simulate that death. So >> whatever happens to git in that case is what we would want to happen >> when we call raise(SIGPIPE). > > That's what I'm talking about. On Windows, you can't raise(SIGPIPE). > You can only raise(Windows_supported_signal) where signal is one of: > SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM as MSDN tells us. Correct. All other signal number should return EINVAL. But, as I said, that does not happen by default. The correct solution is to link against invalidcontinue.obj in the MSVC build. This is a compiler-provided object file that changes the default behavior to the "expected" kind, i.e., C runtime functions return EINVAL when appropriate instead of crashing the application. >> A possibly simpler option would be to just have the MSVC build skip the >> raise() call, and do the exit(141) that comes just after. That is >> probably close enough simulation of SIGPIPE death. Correct. The MinGW build uses an older C runtime library, which does not have the strange default behavior, and we do use that exit(141). And with the fix to the MSVC build suggested above, that version would do likewise. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
On Fri, Mar 28, 2014 at 10:07:22AM +, Marat Radchenko wrote: > > What > > happens if git is still writing data to the pager and the pager exits? > > Does it receive a signal of some sort? > > I'm not sure what you mean, sorry. check_pipe properly detects pager exit. > The problem is with the way it tries to die. Right, but check_pipe shouldn't trigger in most cases on Unix because the process will be killed by SIGPIPE automatically. It's only there to catch the case where we have disabled SIGPIPE. On Windows, what happens to "yes" if you run: yes | (exit 0) On Unix, "yes" receives SIGPIPE and dies. Does it run forever on Windows? If it dies, what does the death look like (does it have a signal death, or exit with a specific code?). > > The point of the code in check_pipe is to simulate that death. So > > whatever happens to git in that case is what we would want to happen > > when we call raise(SIGPIPE). > > That's what I'm talking about. On Windows, you can't raise(SIGPIPE). > You can only raise(Windows_supported_signal) where signal is one of: > SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM as MSDN tells us. Right, I understand that you don't have SIGPIPE. But we want to emulate whatever happens in the case I described above. > > A possibly simpler option would be to just have the MSVC build skip the > > raise() call, and do the exit(141) that comes just after. That is > > probably close enough simulation of SIGPIPE death. > > Isn't raise(SIGTERM/SIGINT) good enough? Perhaps. It is a slight lie. We _didn't_ get a SIGTERM, and anybody looking at our exit code to find out why we died would be misled. But the most important thing is that we die and that the exit status is non-zero. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
Jeff King peff.net> writes: > > I'm not sure what an actual SIGPIPE death looks like on Windows. There is no SIGPIPE death on Windows due to total absence of SIGPIPE. raise(unsupported int) just causes ugly "git.exe has stopped working" window and possibly ends up as SIGABT (I don't know how to check this). > What > happens if git is still writing data to the pager and the pager exits? > Does it receive a signal of some sort? I'm not sure what you mean, sorry. check_pipe properly detects pager exit. The problem is with the way it tries to die. > The point of the code in check_pipe is to simulate that death. So > whatever happens to git in that case is what we would want to happen > when we call raise(SIGPIPE). That's what I'm talking about. On Windows, you can't raise(SIGPIPE). You can only raise(Windows_supported_signal) where signal is one of: SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM as MSDN tells us. > A possibly simpler option would be to just have the MSVC build skip the > raise() call, and do the exit(141) that comes just after. That is > probably close enough simulation of SIGPIPE death. Isn't raise(SIGTERM/SIGINT) good enough? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
On Fri, Mar 28, 2014 at 09:14:07AM +, Marat Radchenko wrote: > > Jeff King peff.net> writes: > > > > > > > > The write_or_die function will always die on an error, > > > including EPIPE. However, it currently treats EPIPE > > > specially by suppressing any error message, and by exiting > > > with exit code 0. > > > > This causes error box on Windows in MSVC=1 build: > > After deeper investigation it turned out that Windows supports > much less signals [1] than POSIX and "If the argument is not a valid signal > as specified above, the invalid parameter handler is invoked". > > The question is - what is the proper way to fix this? > Patch mingw_raise in compat/mingw.c to map unsupported signals into > supported ones like SIGPIPE -> SIGTERM? > > [1]: http://msdn.microsoft.com/en-us/library/dwwzkt4c.aspx I'm not sure what an actual SIGPIPE death looks like on Windows. What happens if git is still writing data to the pager and the pager exits? Does it receive a signal of some sort? The point of the code in check_pipe is to simulate that death. So whatever happens to git in that case is what we would want to happen when we call raise(SIGPIPE). A possibly simpler option would be to just have the MSVC build skip the raise() call, and do the exit(141) that comes just after. That is probably close enough simulation of SIGPIPE death. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
Marat Radchenko slonopotamus.org> writes: > > Jeff King peff.net> writes: > > > > > The write_or_die function will always die on an error, > > including EPIPE. However, it currently treats EPIPE > > specially by suppressing any error message, and by exiting > > with exit code 0. > > This causes error box on Windows in MSVC=1 build: After deeper investigation it turned out that Windows supports much less signals [1] than POSIX and "If the argument is not a valid signal as specified above, the invalid parameter handler is invoked". The question is - what is the proper way to fix this? Patch mingw_raise in compat/mingw.c to map unsupported signals into supported ones like SIGPIPE -> SIGTERM? [1]: http://msdn.microsoft.com/en-us/library/dwwzkt4c.aspx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] MSVC: error box when interrupting `gitlog` by quitting less
Jeff King peff.net> writes: > > The write_or_die function will always die on an error, > including EPIPE. However, it currently treats EPIPE > specially by suppressing any error message, and by exiting > with exit code 0. This causes error box on Windows in MSVC=1 build: git.exe!_invoke_watson(...) Line 132C++ git.exe!_invalid_parameter(...) Line 85 C++ git.exe!_invalid_parameter_noinfo() Line 97 C++ git.exe!raise(int signum) Line 499 C git.exe!mingw_raise(int sig) Line 1745 C git.exe!check_pipe(int err) Line 9 C git.exe!maybe_flush_or_die(_iobuf * f, const char * desc) Line 48 C git.exe!log_tree_commit(rev_info * opt, commit * commit) Line 820 C git.exe!cmd_log_walk(rev_info * rev) Line 344 C git.exe!cmd_log(int argc, const char * * argv, const char * prefix) Line 637 C git.exe!run_builtin(cmd_struct * p, int argc, const char * * argv) Line 314 C git.exe!handle_builtin(int argc, const char * * argv) Line 487 C git.exe!run_argv(int * argcp, const char * * * argv) Line 536 C git.exe!mingw_main(int argc, char * * av) Line 616 C git.exe!main(int argc, char * * argv) Line 551 C "Should never happen", ha-ha. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
