[Twisted-Python] Another tcp.Client question
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
Another question - shouldn't we return when getting a EWOULDBLOK or friends, since we haven't yet connected? ======================== class Client(Connection): #... def doConnect(self): """I connect the socket. Then, call the protocol's makeConnection, and start waiting for data. """ try: self.socket.connect(self.addr) except socket.error, se: if se.args[0] in (EWOULDBLOCK, EALREADY, EINPROGRESS): pass ########### XXX shouldn't this be "return"? XXX else: self.protocol.connectionFailed() return CONNECTION_LOST # If I have reached this point without raising or returning, # that means that the socket is connected.
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
Itamar wrote:
Another question - shouldn't we return when getting a EWOULDBLOK or friends, since we haven't yet connected?
Well, maybe not. But I am having problems - when connecting to a server that is not running using a tcp.Client, instead of getting connectionFailed I'm getting connectionMade and then an immediate connectionLost.
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
OK, the following patch seems to solve the problem. It is based on the fact doWrite is set to equal doConnect - I assume that this patch implements what was intended in the first place. Index: tcp.py =================================================================== RCS file: /cvs/Twisted/twisted/internet/tcp.py,v retrieving revision 1.16 diff -c -r1.16 tcp.py *** tcp.py 2001/08/29 10:13:55 1.16 --- tcp.py 2001/08/31 18:46:49 *************** *** 159,168 **** self.socket.connect(self.addr) except socket.error, se: if se.args[0] in (EWOULDBLOCK, EALREADY, EINPROGRESS): ! pass else: self.protocol.connectionFailed() return CONNECTION_LOST # If I have reached this point without raising or returning, that means # that the socket is connected. del self.doWrite --- 159,170 ---- self.socket.connect(self.addr) except socket.error, se: if se.args[0] in (EWOULDBLOCK, EALREADY, EINPROGRESS): ! self.startWriting() ! return else: self.protocol.connectionFailed() return CONNECTION_LOST + # If I have reached this point without raising or returning, that means # that the socket is connected. del self.doWrite
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
One problem with my patch - both connectionFailed and connectionLost are called on the protocol, instead of just connectionFailed.
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
I just looked at it and I think that this patch (which I'm about to commit, after doing some testing) should solve the problem: diff -u -r1.16 tcp.py --- twisted/internet/tcp.py 2001/08/29 10:13:55 1.16 +++ twisted/internet/tcp.py 2001/09/02 01:58:55 @@ -159,10 +159,10 @@ self.socket.connect(self.addr) except socket.error, se: if se.args[0] in (EWOULDBLOCK, EALREADY, EINPROGRESS): - pass + self.startWriting() else: self.protocol.connectionFailed() - return CONNECTION_LOST + self.stopWriting() # If I have reached this point without raising or returning, that means # that the socket is connected. del self.doWrite @@ -332,7 +332,7 @@ # Since ports can't, by definition, write any data, we can just close # instantly (no need for the more complex stuff for selectables which # write) - removeReader(self) + self.stopReading() self.connectionLost() def connectionLost(self): On Fri, Aug 31, 2001 at 02:46:01PM -0400, Itamar wrote:
-- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/812cc4936ca92a5ad9073f521a32b7fb.jpg?s=120&d=mm&r=g)
Just out of curiousity:
- removeReader(self) + self.stopReading()
That removeReader is _never_ called :) If it were called, the temporary DTP-servers in twisted.ftp would be removed every time I called loseConnection. And it would generate a NameError (I guess), since removeReader resides in main. So, actually this lil' piece of the patch has no effect at all. BTW, what's a good way to close a server? That patch of code does it, but it is overridden elsewhere. // phed
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Sun, Sep 02, 2001 at 10:01:43AM +0200, Benjamin Bruheim wrote:
A quick glance over twisted/protocols/ftp.py indicates to me that nowhere is loseConnection actually called on a Port. Where is it supposed to be? (And what's up with storing it as DTPLoseConnection? I never see that being used...) On my machine, at least, calling loseConnection on a Port *would* indeed cause it to blow up :) -- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/812cc4936ca92a5ad9073f521a32b7fb.jpg?s=120&d=mm&r=g)
No, it was just a test. Calling DTPLoseConnection actually would remove the server since it would call Port.loseConnection, though trigging some unwanted effects. I don't remember the details so don't mind. :) The port is created in FTP.createPassiveServer, and loseConnection is called in DTP.finishGet (or DTP.finish, I don't think I've commited the namechange). Are you sure you get a (Port n closed) on the log? Here they are all shown (successive) first when I shut down. BTW, The point with my last reply was that I can't see that some of the code changed are ever run, and that makes it hard to test ;) // phed
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Mon, Sep 03, 2001 at 12:25:08AM +0200, Benjamin Bruheim wrote:
Is the dtpPort supposed to accept more than one connection? I was under the impression that anywhere one would bind port 0 would be a place that one or only a few connections would be accepted.
I don't remember the details so don't mind. :)
I am willing to bet that the unwanted effects you saw were tracebacks on the code we're talking about :)
The port is created in FTP.createPassiveServer, and loseConnection is called in DTP.finishGet (or DTP.finish, I don't think I've commited the namechange).
You have committed it. Still: I see no loseConnection being called on a *Port*, only on a *Server*, which is a connection that got accepted from a port.
Are you sure you get a (Port n closed) on the log? Here they are all shown (successive) first when I shut down.
That's as a result of seeing connectionLost, loseConnection is never necessarily called.
BTW, The point with my last reply was that I can't see that some of the code changed are ever run, and that makes it hard to test ;)
Yes, and my point is that, as far as I understand FTP, you *should* be exercising that code. I still don't understand why you're not. -- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/812cc4936ca92a5ad9073f521a32b7fb.jpg?s=120&d=mm&r=g)
yes, it shall accept one connection, and after the transfer is done, it shall close the connection (and in this way tell the user that the transfer is done). it shall let the tcp time-out, and that's why we change the port; less time is wasted between each new file. I am thinking of letting the user define a range of ports, and cycle through that instead of using port 0.
Isn't 'self.dtpPort = tcp.Port(0, self)' a Port? self.dtp.transport is self.dtpPort, and its method loseConnection is used. Besides, isn't Port the same as a server?
Well, if I could. And how come nobody ever have run into that removeReader with an NameError or something? // phed
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Mon, Sep 03, 2001 at 12:57:31AM +0200, Benjamin Bruheim wrote:
OK. That isn't really happening. :-)
Yes.
self.dtp.transport is self.dtpPort,
No.
and its method loseConnection is used.
Yes, for some values of "its" :-)
Besides, isn't Port the same as a server?
No. Here's the deal -- tcp.Port creates a _listening socket on a port_. That _listening socket on a Port_ creates a _Server connection_; the class Server represents the byte-stream connection socket, the class Port represents the listening on a port socket. Servers are created by a Port. self.dtp.transport is a twisted.internet.tcp.Server; self.dtpPort is a twisted.internet.tcp.Port (repr 'em sometime ^_^). Both must have their connections lost, at different points. Port.loseConnection() will cause the socket to stop listening. Server.loseConnection() will cause that particular connection to be closed. Port.loseConnection should probably happen in the buildProtocol method of your ProtocolFactory, since it's explicitly only ever supposed to accept one connection. Does this make more sense?
Itamar says he did, I think. I sure did, when I tried it. That particular path of code was never exercised -- although hopefully it will be in FTP fairly soon. twisted.protocols.ftp was never calling it, only Server.loseConnection, which is the source of most of your current troubles, I believe. -- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
Glyph Lefkowitz wrote:
I just looked at it and I think that this patch (which I'm about to commit, after doing some testing) should solve the problem:
I'll test it out as well, if not today (I'm leaving to the airport in two hours) then on Monday. I'm not sure it will work however - the logic after the socket.connect exception handler assumes that if you've reched that point you are either connected or not (as the comment there says). So you'd want to return after the startWriting(). And what are the changes to Port for?
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
Itamar wrote:
Another question - shouldn't we return when getting a EWOULDBLOK or friends, since we haven't yet connected?
Well, maybe not. But I am having problems - when connecting to a server that is not running using a tcp.Client, instead of getting connectionFailed I'm getting connectionMade and then an immediate connectionLost.
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
OK, the following patch seems to solve the problem. It is based on the fact doWrite is set to equal doConnect - I assume that this patch implements what was intended in the first place. Index: tcp.py =================================================================== RCS file: /cvs/Twisted/twisted/internet/tcp.py,v retrieving revision 1.16 diff -c -r1.16 tcp.py *** tcp.py 2001/08/29 10:13:55 1.16 --- tcp.py 2001/08/31 18:46:49 *************** *** 159,168 **** self.socket.connect(self.addr) except socket.error, se: if se.args[0] in (EWOULDBLOCK, EALREADY, EINPROGRESS): ! pass else: self.protocol.connectionFailed() return CONNECTION_LOST # If I have reached this point without raising or returning, that means # that the socket is connected. del self.doWrite --- 159,170 ---- self.socket.connect(self.addr) except socket.error, se: if se.args[0] in (EWOULDBLOCK, EALREADY, EINPROGRESS): ! self.startWriting() ! return else: self.protocol.connectionFailed() return CONNECTION_LOST + # If I have reached this point without raising or returning, that means # that the socket is connected. del self.doWrite
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
One problem with my patch - both connectionFailed and connectionLost are called on the protocol, instead of just connectionFailed.
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
I just looked at it and I think that this patch (which I'm about to commit, after doing some testing) should solve the problem: diff -u -r1.16 tcp.py --- twisted/internet/tcp.py 2001/08/29 10:13:55 1.16 +++ twisted/internet/tcp.py 2001/09/02 01:58:55 @@ -159,10 +159,10 @@ self.socket.connect(self.addr) except socket.error, se: if se.args[0] in (EWOULDBLOCK, EALREADY, EINPROGRESS): - pass + self.startWriting() else: self.protocol.connectionFailed() - return CONNECTION_LOST + self.stopWriting() # If I have reached this point without raising or returning, that means # that the socket is connected. del self.doWrite @@ -332,7 +332,7 @@ # Since ports can't, by definition, write any data, we can just close # instantly (no need for the more complex stuff for selectables which # write) - removeReader(self) + self.stopReading() self.connectionLost() def connectionLost(self): On Fri, Aug 31, 2001 at 02:46:01PM -0400, Itamar wrote:
-- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/812cc4936ca92a5ad9073f521a32b7fb.jpg?s=120&d=mm&r=g)
Just out of curiousity:
- removeReader(self) + self.stopReading()
That removeReader is _never_ called :) If it were called, the temporary DTP-servers in twisted.ftp would be removed every time I called loseConnection. And it would generate a NameError (I guess), since removeReader resides in main. So, actually this lil' piece of the patch has no effect at all. BTW, what's a good way to close a server? That patch of code does it, but it is overridden elsewhere. // phed
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Sun, Sep 02, 2001 at 10:01:43AM +0200, Benjamin Bruheim wrote:
A quick glance over twisted/protocols/ftp.py indicates to me that nowhere is loseConnection actually called on a Port. Where is it supposed to be? (And what's up with storing it as DTPLoseConnection? I never see that being used...) On my machine, at least, calling loseConnection on a Port *would* indeed cause it to blow up :) -- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/812cc4936ca92a5ad9073f521a32b7fb.jpg?s=120&d=mm&r=g)
No, it was just a test. Calling DTPLoseConnection actually would remove the server since it would call Port.loseConnection, though trigging some unwanted effects. I don't remember the details so don't mind. :) The port is created in FTP.createPassiveServer, and loseConnection is called in DTP.finishGet (or DTP.finish, I don't think I've commited the namechange). Are you sure you get a (Port n closed) on the log? Here they are all shown (successive) first when I shut down. BTW, The point with my last reply was that I can't see that some of the code changed are ever run, and that makes it hard to test ;) // phed
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Mon, Sep 03, 2001 at 12:25:08AM +0200, Benjamin Bruheim wrote:
Is the dtpPort supposed to accept more than one connection? I was under the impression that anywhere one would bind port 0 would be a place that one or only a few connections would be accepted.
I don't remember the details so don't mind. :)
I am willing to bet that the unwanted effects you saw were tracebacks on the code we're talking about :)
The port is created in FTP.createPassiveServer, and loseConnection is called in DTP.finishGet (or DTP.finish, I don't think I've commited the namechange).
You have committed it. Still: I see no loseConnection being called on a *Port*, only on a *Server*, which is a connection that got accepted from a port.
Are you sure you get a (Port n closed) on the log? Here they are all shown (successive) first when I shut down.
That's as a result of seeing connectionLost, loseConnection is never necessarily called.
BTW, The point with my last reply was that I can't see that some of the code changed are ever run, and that makes it hard to test ;)
Yes, and my point is that, as far as I understand FTP, you *should* be exercising that code. I still don't understand why you're not. -- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/812cc4936ca92a5ad9073f521a32b7fb.jpg?s=120&d=mm&r=g)
yes, it shall accept one connection, and after the transfer is done, it shall close the connection (and in this way tell the user that the transfer is done). it shall let the tcp time-out, and that's why we change the port; less time is wasted between each new file. I am thinking of letting the user define a range of ports, and cycle through that instead of using port 0.
Isn't 'self.dtpPort = tcp.Port(0, self)' a Port? self.dtp.transport is self.dtpPort, and its method loseConnection is used. Besides, isn't Port the same as a server?
Well, if I could. And how come nobody ever have run into that removeReader with an NameError or something? // phed
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Mon, Sep 03, 2001 at 12:57:31AM +0200, Benjamin Bruheim wrote:
OK. That isn't really happening. :-)
Yes.
self.dtp.transport is self.dtpPort,
No.
and its method loseConnection is used.
Yes, for some values of "its" :-)
Besides, isn't Port the same as a server?
No. Here's the deal -- tcp.Port creates a _listening socket on a port_. That _listening socket on a Port_ creates a _Server connection_; the class Server represents the byte-stream connection socket, the class Port represents the listening on a port socket. Servers are created by a Port. self.dtp.transport is a twisted.internet.tcp.Server; self.dtpPort is a twisted.internet.tcp.Port (repr 'em sometime ^_^). Both must have their connections lost, at different points. Port.loseConnection() will cause the socket to stop listening. Server.loseConnection() will cause that particular connection to be closed. Port.loseConnection should probably happen in the buildProtocol method of your ProtocolFactory, since it's explicitly only ever supposed to accept one connection. Does this make more sense?
Itamar says he did, I think. I sure did, when I tried it. That particular path of code was never exercised -- although hopefully it will be in FTP fairly soon. twisted.protocols.ftp was never calling it, only Server.loseConnection, which is the source of most of your current troubles, I believe. -- ______ __ __ _____ _ _ | ____ | \_/ |_____] |_____| |_____| |_____ | | | | @ t w i s t e d m a t r i x . c o m http://twistedmatrix.com/users/glyph
![](https://secure.gravatar.com/avatar/433365de0f787faa3ed3e6dd1da5884f.jpg?s=120&d=mm&r=g)
Glyph Lefkowitz wrote:
I just looked at it and I think that this patch (which I'm about to commit, after doing some testing) should solve the problem:
I'll test it out as well, if not today (I'm leaving to the airport in two hours) then on Monday. I'm not sure it will work however - the logic after the socket.connect exception handler assumes that if you've reched that point you are either connected or not (as the comment there says). So you'd want to return after the startWriting(). And what are the changes to Port for?
participants (3)
-
Benjamin Bruheim
-
Glyph Lefkowitz
-
Itamar