classification
Title: asyncore/asynchat patches
Type: Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: josiahcarlson Nosy List: alanmcintyre, brett.cannon, djarb, giampaolo.rodola, gvanrossum, intgr, josiahcarlson, svncodereview
Priority: normal Keywords: patch

Created on 2007-06-13 00:37 by josiahcarlson, last changed 2009-03-28 12:22 by intgr. This issue is now closed.

Files
File name Uploaded Description Edit
async_patch2.patch josiahcarlson, 2007-06-13 00:37 async sockets patch for 2.6
initiate_send.py giampaolo.rodola, 2007-12-11 01:20
differences.diff giampaolo.rodola, 2007-12-11 01:20
patch.diff giampaolo.rodola, 2008-03-02 23:22 Updated patch (does not include test_suite and doc changes provided in first place since they're out of date)
full_async_patch.patch giampaolo.rodola, 2008-05-04 18:26
Messages (16)
msg52765 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2007-06-13 00:37
There are a handful of outstanding asyncore/asynchat related bugs in the tracker.  This patch seeks to handle all of the issues with the exception of windows-specific socket errors from 658749 .

In particular, it takes pieces of patches 909005 and 1736101, to address some issues raised there, as well as the bugs: 539444, 760475, 777588, 889153, 953599, 1025525, and 1063924.

In some cases where it could potentially handle fewer exceptions (see the send method), I have opted to handle more of them due to testing actually producing them.

It adds test cases from patch 909005.

It adds two new sample methods to asynchat.async_chat _get_data and _collect_incoming_data to be used as examples for handling incoming buffers.

It removes the simple_producer and fifo classes from asynchat, but accepts work-alikes to the asynchat.async_chat.push_with_producer method.

It further fixes an unreported violated invariant in asynchat.async_chat.handle_read() .

It also produces a useful error message when asyncore.dispatcher.handle_expt_evt is called if the base handle_expt has not been overwritten.


This patch should be applied to the trunk.  The new methods, and removal of the two classes should not be included in patches to 2.5 maintenance (don't add the methods and don't remove the classes).  Basically everything else should work as it did before (with better error handling), unless someone was monkey-patching asyncore.dispatcher.handle_expt .
msg52766 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2007-07-11 15:16
Good work. I would only rencommend to include documentation for asyncore.close_all function, actually not documented yet.
msg52767 - (view) Author: Alan McIntyre (alanmcintyre) * (Python committer) Date: 2007-08-08 16:09
This patch removes two public--and documented--classes from asynchat (simple_producer and fifo).  Even if the proposed changes to asynchat make them utterly worthless, I thought it was general policy to deprecate public items for a release or two before removing them.  Is this the case even when they're as simple as these two classes?
msg52768 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2007-08-12 22:30
We can leave those two classes in, even though I have never seen them used or subclassed.  In Python 3.x, we can remove them without issue.
msg57581 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2007-11-16 02:45
The current implementation of asynchat.async_chat.initiate_send method
doesn't look at what is specified in ac_out_buffer_size attribute which
represents the buffer size of the outgoing data defaulting to a maximum
of 4096 bytes to send in a single socket.send() call.

Note that this only happens when sending the data by using a producer
through the use of the push_with_producer method.
This happens because while the older asynchat version used slicing for
buffering:

> num_sent = self.send(self.ac_out_buffer[:obs]) # obs == ac_out_buffer_size

...the newer version just calls self.send using the entire data as
argument without caring of what ac_out_buffer_size thinks about it:

> num_sent = self.send(first)

What is specified in ac_out_buffer_size when using a producer is just
ignored and the only way to have control over the outgoing data buffer
is to operate directly on the producer.
msg58400 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2007-12-11 01:20
In attachment I provide a patch for fixing this last mentioned issue.
It's a rewriting of initiate_send method which now looks at what is
specified by ac_out_buffer_size attribute.
msg63193 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-03-02 23:22
I've discussed a lot with Josiah via e-mail and this is the updated
version of the patch including a fix for the two issues raised before.
This update has been needed also because the original patch has been
out-dated by some commits after r53734 involving the test suite
and the documentation, which I both took off this patch.
I also re-added simple_producer and fifo classes in asynchat.py since it
seems they are needed for passing tests.

The test suite has passed on Windows XP using Python 2.6 alpha 1.
I have also successfully run the test suite of a project of mine based
on asynchat which includes over 40 tests.
msg64437 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-03-24 19:40
Are there news about this issue?
msg66230 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-05-04 18:26
In attachment the Josiah Carlson's patch who currently seems to have
problems in accessing the bug tracker.
It should address all the problems raised on python-dev related discussion:
http://groups.google.com/group/python-dev2/browse_thread/thread/eec1ddadefe09fd8/40e79742231076b9?lnk=gst&q=asyncore#40e79742231076b9
msg66231 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-05-04 19:22
This is separate from issue1563, right?
msg66234 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-05-04 19:43
Yes.
msg66296 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-05-05 22:02
FWIW, I've added Giampaolo's latest patch to Rietveld:
http://codereview.appspot.com/744

Review comments added there should automatically be CC'ed here.
msg66297 - (view) Author: (svncodereview) Date: 2008-05-05 22:02
Dear report@bugs.python.org,

New code review comments by gvanrossum@gmail.com have been published.
Please go to http://codereview.appspot.com/744 to read them.

Message:
(This is mostly a test of the bug/rietveld integration.)

Details:

http://codereview.appspot.com/744/diff/1/22
File Doc/library/asyncore.rst (right):

http://codereview.appspot.com/744/diff/1/22#newcode226
Line 226: A file_dispatcher takes a file descriptor or file object 
along with an optional
Mind keeping the line length under 80 chars?

Issue Description:
http://bugs.python.org/issue1736190

Sincerely,

  Your friendly code review daemon (http://codereview.appspot.com/).
msg67168 - (view) Author: (svncodereview) Date: 2008-05-21 19:09
Dear GvR, report,

New code review comments by josiah.carlson have been published.
Please go to http://codereview.appspot.com/744 to read them.

Message:

Details:

http://codereview.appspot.com/744/diff/1/22
File Doc/library/asyncore.rst (right):

http://codereview.appspot.com/744/diff/1/22#newcode226
Line 226: A file_dispatcher takes a file descriptor or file object 
along with an optional
On 2008/05/05 22:02:22, GvR wrote:
> Mind keeping the line length under 80 chars?

No problem.  Any other comments?

Issue Description:
http://bugs.python.org/issue1736190

Sincerely,

  Your friendly code review daemon (http://codereview.appspot.com/).
msg69223 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2008-07-03 18:14
Committed to trunk a bit ago, will be in 3.0 this weekend.
msg74126 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-10-01 11:39
This issue should be closed.
History
Date User Action Args
2009-03-28 12:22:27intgrsetnosy: + intgr
2008-10-01 11:39:00giampaolo.rodolasetmessages: + msg74126
2008-07-03 18:14:57josiahcarlsonsetstatus: open -> closed
resolution: accepted
messages: + msg69223
2008-05-21 19:09:54svncodereviewsetmessages: + msg67168
2008-05-05 22:02:26svncodereviewsetnosy: + svncodereview
messages: + msg66297
2008-05-05 22:02:01gvanrossumsetmessages: + msg66296
2008-05-04 19:43:00giampaolo.rodolasetmessages: + msg66234
2008-05-04 19:22:36brett.cannonsetnosy: + brett.cannon
messages: + msg66231
2008-05-04 18:26:13giampaolo.rodolasetfiles: + full_async_patch.patch
messages: + msg66230
2008-04-08 00:20:32gvanrossumsetnosy: gvanrossum, josiahcarlson, alanmcintyre, giampaolo.rodola, djarb
2008-03-24 19:40:56giampaolo.rodolasetnosy: + gvanrossum
messages: + msg64437
2008-03-02 23:22:47giampaolo.rodolasetfiles: + patch.diff
messages: + msg63193
2008-02-14 16:41:43djarbsetnosy: + djarb
2008-01-23 19:17:37giampaolo.rodolasettype: security ->
severity: urgent -> normal
versions: + Python 2.6, - Python 2.5
2008-01-23 19:16:58giampaolo.rodolasettype: security
severity: normal -> urgent
versions: + Python 2.5
2007-12-11 01:20:47giampaolo.rodolasetfiles: + differences.diff
2007-12-11 01:20:34giampaolo.rodolasetfiles: + initiate_send.py
messages: + msg58400
2007-11-16 02:45:19giampaolo.rodolasetmessages: + msg57581
2007-06-13 00:37:23josiahcarlsoncreate