Patches item #1519025, was opened at 2006-07-07 16:02 Message generated for change (Comment added) made by nnorwitz You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1519025&group_id=5470
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: Modules Group: None Status: Open Resolution: None Priority: 8 Submitted By: Tony Nelson (tony_nelson) Assigned to: Anthony Baxter (anthonybaxter) Summary: New ver. of 1102879: Fix for 926423: socket timeouts Initial Comment: This is a new version of the patch for 1102879, "Fix for 926423: socket timeouts + Ctrl-C don't play nice". It passes "make EXTRATESTOPTS=-unetwork test". I've also made a version for Python 2.4.3 (for FC5), at <http://georgeanelson.com/socketctlc24.patch>, which also passes tests, and works with yum on FC5. This is my first patch to Python. I didn't see a way to attach a file to the old patch. My patch is slightly different from the original patch. My patch brings the error path back to the main line: where the original patch would call PyErr_SetFromErrno() for internal_select() errors, mine initializes the status vars so that the normal error handler will be called, and detects timeout when internal_select() returns 1. Thus the normal (replaceable) error handler is called for errors whether or not a timeout is set. The patch handles connect() calls, and sock_connect_ex() now checks for signals as would PyErr_SetFromErrno(). I didn't change the name "timeout" as it didn't bother me. I didn't add any confusing macros. This patch took so long because of confusion (and vacation). Before the patch, Ctl-C would produce an EWOULDBLOCK error, after the patch an EINTR error, but still no KeyboardInterrupt exception. It too me way too long to notice that the rpm library used by yum sets its own signal handlers, and that one line of code (setting SIGINT back to the python default) would fix it. Now that I have KeyboardInterrupts I have confidence in this patch. ---------------------------------------------------------------------- >Comment By: Neal Norwitz (nnorwitz) Date: 2006-07-29 11:23 Message: Logged In: YES user_id=33168 Does the bug affect Windows? A test that only runs on Unix is better than nothing. If the test can't be fixed to run on Windows, please add a comment with an XXX to discuss this and try to get someone to impl a Windows version. ---------------------------------------------------------------------- Comment By: Tony Nelson (tony_nelson) Date: 2006-07-29 07:36 Message: Logged In: YES user_id=1356214 Darn. My test will only work on *nix, and not on other platforms such as MSWindows. I don't see a way to send a signal that would work on all platforms, so I don't know a way to test the patch. I'll have to ask for help. The test may need to be changed to only test on *nix. ---------------------------------------------------------------------- Comment By: Tony Nelson (tony_nelson) Date: 2006-07-28 20:46 Message: Logged In: YES user_id=1356214 Here is a new patch that adds a test, in test_socket.py TCPTimeoutTest.testInterruptedTimeout(); the functional part of the patch is unchanged. The test doesn't use an actual Ctl-C, or even a SIGINT, as sending one of them seemed hard; it sends a SIGALRM instead. The test passes with the patch; without the patch it prints a confused error message, but unfortunately that's about right. As for what I mean by "test", well, I was confused about what was being threaded. I have it straight now. ---------------------------------------------------------------------- Comment By: Neal Norwitz (nnorwitz) Date: 2006-07-24 20:58 Message: Logged In: YES user_id=33168 The test_unary_minus problem is due to gcc4.1 optimizations. We need to fix that. Yes, please write a test. As long as you can reproduce the problem with the old code and the new code doesn't crash, that's an improvement. Thanks! Not sure what you mean by "test". It should at least be in a separate method. Perhaps you could use a new class in test_socket. I doubt that a new file should be necessary. Even a new class seems like it probably shouldn't be necessary, but I can't say without actually having written the test. We'll figure out where to put it as long as we have a test case. ---------------------------------------------------------------------- Comment By: Tony Nelson (tony_nelson) Date: 2006-07-23 18:58 Message: Logged In: YES user_id=1356214 Thank you for your advice. I have updated the patch for HEAD (r50793). It passes "make EXTRATESTOPTS=-unetwork test" except for test_compile, which fails test_unary_minus. Should I try to write the test? I think that it could try to accept() and another thread could send a signal. I think it would have to be separate from all the other tests, which are threaded, so that it won't interfere with them. ---------------------------------------------------------------------- Comment By: Neal Norwitz (nnorwitz) Date: 2006-07-23 11:36 Message: Logged In: YES user_id=33168 I'm +0.5 or more this going in to 2.5 final. However, I'd really like to see a test for this. Also, the patch should be updated to HEAD. It doesn't look like it will apply cleanly as there were many changes to socketmodule.c. As for a test, I think if you try to connect to a non-existant host and send a signal from another thread, I think that can trigger the bug. Anthony, let me know your thoughts. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1519025&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
