[Patches] [ python-Patches-1519025 ] New ver. of 1102879: Fix for 926423: socket timeouts

[SourceForge.net]

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: 5
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-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