This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: bug in asyncore.dispatcher_with_send
Type: behavior Stage:
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: adamhj, giampaolo.rodola
Priority: normal Keywords:

Created on 2012-02-03 03:59 by adamhj, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
bug-emu.py adamhj, 2012-02-04 13:00
Messages (7)
msg152494 - (view) Author: adamhj (adamhj) Date: 2012-02-03 03:59
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
msg152503 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2012-02-03 13:18
> 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?
msg152607 - (view) Author: adamhj (adamhj) Date: 2012-02-04 11:21
> 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
msg152618 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2012-02-04 13:00
> 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.
msg152619 - (view) Author: adamhj (adamhj) Date: 2012-02-04 13:00
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
msg152620 - (view) Author: adamhj (adamhj) Date: 2012-02-04 13:31
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
msg152790 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2012-02-06 22:08
No problem.
History
Date User Action Args
2022-04-11 14:57:26adminsetgithub: 58136
2012-02-06 22:08:14giampaolo.rodolasetmessages: + msg152790
2012-02-04 13:31:32adamhjsetstatus: open -> closed

messages: + msg152620
2012-02-04 13:00:17adamhjsetfiles: + bug-emu.py

messages: + msg152619
2012-02-04 13:00:01giampaolo.rodolasetmessages: + msg152618
2012-02-04 11:21:41adamhjsetmessages: + msg152607
2012-02-03 13:18:09giampaolo.rodolasetnosy: + giampaolo.rodola
messages: + msg152503
2012-02-03 03:59:59adamhjcreate