bug in asyncore.dispatcher_with_send

[adamhj]

BPO	13928
Nosy	@giampaolo
Files	bug-emu.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-02-04.13:31:32.642>
created_at = <Date 2012-02-03.03:59:58.991>
labels = ['type-bug', 'library']
title = 'bug in asyncore.dispatcher_with_send'
updated_at = <Date 2012-02-06.22:08:14.799>
user = 'https://bugs.python.org/adamhj'

bugs.python.org fields:

activity = <Date 2012-02-06.22:08:14.799>
actor = 'giampaolo.rodola'
assignee = 'none'
closed = True
closed_date = <Date 2012-02-04.13:31:32.642>
closer = 'adamhj'
components = ['Library (Lib)']
creation = <Date 2012-02-03.03:59:58.991>
creator = 'adamhj'
dependencies = []
files = ['24417']
hgrepos = []
issue_num = 13928
keywords = []
message_count = 7.0
messages = ['152494', '152503', '152607', '152618', '152619', '152620', '152790']
nosy_count = 2.0
nosy_names = ['giampaolo.rodola', 'adamhj']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue13928'
versions = ['Python 2.7']

[adamhj]

i found 2 relative bugs in asyncore.dispatcher_with_send class:

one is in the asyncore.dispatcher_with_send.writable():
def writable(self):
return (not self.connected) or len(self.out_buffer)
why is a not connected connection writable? i think this is definitely a bug
my fix:
def writable(self):
return self.connected and len(self.out_buffer)

another bug is more obscure, i'm not sure is it a bug or something should be handled by user(programmer)

the bug is also in asyncore.dispatcher_with_send class, and maybe also in asyncore.dispatcher class. asyncore.dispatcher uses unblocking socket to handle network missions, when we use the connect() method of dispatcher to establish the socket, it will call socket.connect_ex() method to create the connection, however, socket.connect_ex() may return 10035(EWOULDBLOCK) as it is an unblocking socket indicates that the connection creating is not finished yet, if we call dispatcher.connect() immediately after .connect(), socket error 10057 may be raised, indicating that the socket is not established yet, then the asyncore main loop catches this exception, and calls handle_error(in my case i close the connection in handle_error so the connection which would be established with no problem breaks), i think there should be a connection state check in asyncore.dispatcher.send(), or at least in asyncore.dispatcher_with_send.send.

my fix for asyncore.dispatcher_with_send.send():
def send(self, data):
if self.debug:
self.log_info('sending %s' % repr(data))
self.out_buffer = self.out_buffer + data
if self.connected: # do not call send() if connection is
self.initiate_send() # not established yet, just put data
# in buffer

for the second bug, to reproduce it, just create a unblocking socket to a remote, long delay port with socket.connect_ex and call send immediately

[giampaolo]

why is a not connected connection writable?

A non connected socket must be writable in order to connect.

if we call dispatcher.connect() immediately after .connect(),
socket error 10057 may be raised,

Not sure what you mean here. Why would you call connect() twice?

[adamhj]

A non connected socket must be writable in order to connect.
i can't understand this, does it means that one may use self.connect() in handle_write()? and in fact i found something seems opposite on this page: http://docs.python.org/howto/sockets.html

"If a socket is in the output readable list, you can be as-close-to-certain-as-we-ever-get-in-this-business that a recv on that socket will return something. Same idea for the writable list. You’ll be able to send something."

"If you have created a new socket to connect to someone else, put it in the potential_writers list. If it shows up in the writable list, you have a decent chance that it has connected."

from the latter paragraph may i assume that a writable socket should always has been connected?

Not sure what you mean here. Why would you call connect() twice?
sorry for the typo, it should be "if we call dispatcher.send() immediately after .connect(), socket error 10057 may be raised", this happens if you connect to a high delay remote port, when the .send() is called before the SYN ACK is received.

i think it maybe the programmer's responsibility to check the connection availability when using dispatcher class, but at least for dispatcher_with_send class, programmer should not need do anything special to use send() after a successful connect(), if .send() is called before connection established successfully, send() should only buffer the sending data and wait the socket to become writable

by the way, i found this behavior on windows 7 x32/python 2.7.2

[giampaolo]

i can't understand this, does it means that one may use
self.connect() in handle_write()?

Nope. When handle_write() is called you are supposed to be *already* connected, hence there's no point in calling connect() again.
This is clear if you look at handle_write_event method or read asyncore doc.

"If it shows up in the writable list, you have a decent
"chance that it has connected."
from the latter paragraph may i assume that a writable
socket should always has been connected?

Nope. It means that *before* showing up the socket was *not* connected.

if we call dispatcher.send() immediately after .connect(),
socket error 10057 may be raised",

Of course it does: you're not connected yet (10057 = WSAENOTCONN).
You're supposed to use send() in handle_connect(), when the connection has already been established.
This:

     self.connect()
     self.send('hello')

...is not asyncore is supposed to be used.

[adamhj]

here is a script emulating what is happened in dispatcher class when the second bug triggered. you can use either a non-exist host/port, or a high delay remote port as target(see the comments in the script) and you may use a sniffer(tcpdump/wireshark e.g.) with the script to see what is happen

[adamhj]

ah just ignore my previous msg as i post it without seeing yours.

Nope. It means that *before* showing up the socket was *not* connected.
ok, i read more in the asyncore source and finally understand what do you mean by "A non connected socket must be writable in order to connect". i think this is a little confusing if without any explaining, shouldn't this be written into the asyncore reference page as both readable() and writable() may be override by user?

Of course it does: you're not connected yet (10057 = WSAENOTCONN).
You're supposed to use send() in handle_connect(), when the connection has already been established.
This:

self.connect()
self.send('hello')

...is not asyncore is supposed to be used.

oh it seems i forgot handle_connect(), thank you for your patient explanation and sorry to have troubled you. i think there is no problem anymore

[giampaolo]

No problem.