Re: [Twisted-Python] connectionLost never reached after calling loseConnection: stuck in CLOSE_WAIT forever
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Hello Jean-Paul, thanks for tracking this down, you rock! I promise that when I'll have payed all by debts I'll buy one of those posters of Exarkun to hang on my wall! JP wrote:
Following your indications I attached a minimal example to a new ticket: http://twistedmatrix.com/trac/ticket/4719 Some additional info: * problem occurs only if more than 64KB of data are written to the transport before its consumer calls stopProducing on it * problem occurs only if some time passes before its producer unregisters itself from the transport Thanks again for your help! :) ste
![](https://secure.gravatar.com/avatar/d7875f8cfd8ba9262bfff2bf6f6f9b35.jpg?s=120&d=mm&r=g)
On Sat, 2010-10-30 at 18:48 +0200, Stefano Debenedetti wrote:
You can also get figurines: http://www.toycutter.com/2009/02/exar-kun-three-eye-monster-munnys.html
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Il 30/10/2010 18:48, Stefano Debenedetti ha scritto:
I have attached a 2 lines patch that seems to fix the issue. The patch also contains a 50 lines unit test ;) I hope this makes it on time for the 10.2.0 release. BTW there's a small error here: http://twistedmatrix.com/trac/wiki/BasicGuideToContributingCode $ svn diff -u Subcommand 'diff' doesn't accept option '-u [--show-updates]' Type 'svn help diff' for usage. So the correct command for generating a patch should be documented as: svn diff > my-twisted-patch.patch thanks ciao ste
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Oct 31, 2010, at 1:36 PM, Stefano Debenedetti wrote:
Stefano, this looks like a great contribution: a serious bug fix in a bit of core functionality, with a test. Thanks so much! I'll try to do a complete review soon, but my immediate impression is that test_disconnectEvent needs a better docstring. The word 'correctly' in a docstring is always a clue that the writer gave up and stopped trying to describe what 'correct' behavior is :). if you could try to rephrase it as "When a transport is (...) and then (...), (...) should be called on (...)". (I will have a more concrete suggestion, with fewer "..."s when I have time to go through the ticket history and review the patch in detail.)
![](https://secure.gravatar.com/avatar/73d971b6afe2f372e17b66a77b0eb4cc.jpg?s=120&d=mm&r=g)
Hey, Inside an twisted.words.protocol.irc.IRCClient class I want to download via XDCC and then have a callback after that's finished. Sadly the documentation is lacking and I'm not sure what is what. If I override IRCClient.dccDoSend(self, user, address, port, filename, size, data) then it enters when I initiate an XDCC SEND request. But how do I start the download? 1- How can I start downloading the file?2- Can I receive an array of the bytes instead of saving a file?3- How can I be notified when the download is complete? Thanks
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Glyph wrote:
Stefano, this looks like a great contribution: a serious bug fix in a bit of core functionality, with a test. Thanks so much!
Thank you! It's a pleasure to have one's code be called by code in the Twisted framework and to get stuff done with it! And it's an honor to receive support from such a wonderful community, already ever so enjoyable and enlightening even when just lurking among it!
I'll try to do a complete review soon, but my immediate impression is that test_disconnectEvent needs a better docstring. The word 'correctly' in a docstring is always a clue that the writer gave up and stopped trying to describe what 'correct' behavior is :).
Good point, I totally agree with you. "correctly" is most often the wrong word and a shortcut which leads to unpleasant consequences once what is "correct" gets better defined later by explicitly broken behavior. BTW that line was actually shamelessly copy-pasted from the previous test's docstring ;P
if you could try to rephrase it as "When a transport is (...) and then (...), (...) should be called on (...)".
(I will have a more concrete suggestion, with fewer "..."s when I have time to go through the ticket history and review the patch in detail.)
OK, take two: This test checks that a protocol gets notified of a lost connection when its transport has a registered producer writing more than 64KB to it and is then asked to stopProducing without any more data being written to it. This ought to prevent that sockets get stuck in CLOSE_WAIT state when these peculiar conditions occur. See #4719. Another thing I'm not too sure about is if TCPClientTestsBuilder ("Builder defining tests relating to L{IReactorTCP.connectTCP}.") is the right class for holding such a test, which is not very related to connectTCP. ciao thanks ste
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Il 31/10/2010 18:36, Stefano Debenedetti ha scritto:
err, it seems to fix that issue but it introduces another issue: File "my/code.py", line 759, in loseConnection proto.transport.unregisterProducer() File "/home/lala/lib/python/twisted/internet/abstract.py", line 330, in unregisterProducer self.startWriting() File "/home/lala/lib/python/twisted/internet/abstract.py", line 290, in startWriting self.reactor.addWriter(self) File "/home/lala/lib/python/twisted/internet/epollreactor.py", line 107, in addWriter self._add(writer, self._writes, self._reads, self._selectables, _epoll.OUT, _epoll.IN) File "/home/lala/lib/python/twisted/internet/epollreactor.py", line 88, in _add self._poller._control(cmd, fd, flags) File "_epoll.pyx", line 125, in _epoll.epoll._control I just saw this appearing once in the logs of my app after I applied the patch I suggested: http://twistedmatrix.com/trac/attachment/ticket/4719/avoid_hang_in_CLOSE_WAI... I'll try to replicate and to figure out what does FileDescriptor.unregisterProducer need to do instead of the probably too dumb lines I added there: if self.disconnecting: self.startWriting() ciao ste
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Il 01/11/2010 15:34, Stefano Debenedetti ha scritto:
Alright, I added a test case that replicates the new error introduced by the above change and included it in a new proposed fix: http://twistedmatrix.com/trac/attachment/ticket/4719/avoid_hang_in_CLOSE_WAI... if self.connected and self.disconnecting: self.startWriting() This new patch now passes both new unit tests. Sorry about the noise, if I find any more issues about this I'll post them to the ticket page instead of polluting the mailing list with replies to myself. ciao ste
![](https://secure.gravatar.com/avatar/d7875f8cfd8ba9262bfff2bf6f6f9b35.jpg?s=120&d=mm&r=g)
On Sat, 2010-10-30 at 18:48 +0200, Stefano Debenedetti wrote:
You can also get figurines: http://www.toycutter.com/2009/02/exar-kun-three-eye-monster-munnys.html
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Il 30/10/2010 18:48, Stefano Debenedetti ha scritto:
I have attached a 2 lines patch that seems to fix the issue. The patch also contains a 50 lines unit test ;) I hope this makes it on time for the 10.2.0 release. BTW there's a small error here: http://twistedmatrix.com/trac/wiki/BasicGuideToContributingCode $ svn diff -u Subcommand 'diff' doesn't accept option '-u [--show-updates]' Type 'svn help diff' for usage. So the correct command for generating a patch should be documented as: svn diff > my-twisted-patch.patch thanks ciao ste
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Oct 31, 2010, at 1:36 PM, Stefano Debenedetti wrote:
Stefano, this looks like a great contribution: a serious bug fix in a bit of core functionality, with a test. Thanks so much! I'll try to do a complete review soon, but my immediate impression is that test_disconnectEvent needs a better docstring. The word 'correctly' in a docstring is always a clue that the writer gave up and stopped trying to describe what 'correct' behavior is :). if you could try to rephrase it as "When a transport is (...) and then (...), (...) should be called on (...)". (I will have a more concrete suggestion, with fewer "..."s when I have time to go through the ticket history and review the patch in detail.)
![](https://secure.gravatar.com/avatar/73d971b6afe2f372e17b66a77b0eb4cc.jpg?s=120&d=mm&r=g)
Hey, Inside an twisted.words.protocol.irc.IRCClient class I want to download via XDCC and then have a callback after that's finished. Sadly the documentation is lacking and I'm not sure what is what. If I override IRCClient.dccDoSend(self, user, address, port, filename, size, data) then it enters when I initiate an XDCC SEND request. But how do I start the download? 1- How can I start downloading the file?2- Can I receive an array of the bytes instead of saving a file?3- How can I be notified when the download is complete? Thanks
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Glyph wrote:
Stefano, this looks like a great contribution: a serious bug fix in a bit of core functionality, with a test. Thanks so much!
Thank you! It's a pleasure to have one's code be called by code in the Twisted framework and to get stuff done with it! And it's an honor to receive support from such a wonderful community, already ever so enjoyable and enlightening even when just lurking among it!
I'll try to do a complete review soon, but my immediate impression is that test_disconnectEvent needs a better docstring. The word 'correctly' in a docstring is always a clue that the writer gave up and stopped trying to describe what 'correct' behavior is :).
Good point, I totally agree with you. "correctly" is most often the wrong word and a shortcut which leads to unpleasant consequences once what is "correct" gets better defined later by explicitly broken behavior. BTW that line was actually shamelessly copy-pasted from the previous test's docstring ;P
if you could try to rephrase it as "When a transport is (...) and then (...), (...) should be called on (...)".
(I will have a more concrete suggestion, with fewer "..."s when I have time to go through the ticket history and review the patch in detail.)
OK, take two: This test checks that a protocol gets notified of a lost connection when its transport has a registered producer writing more than 64KB to it and is then asked to stopProducing without any more data being written to it. This ought to prevent that sockets get stuck in CLOSE_WAIT state when these peculiar conditions occur. See #4719. Another thing I'm not too sure about is if TCPClientTestsBuilder ("Builder defining tests relating to L{IReactorTCP.connectTCP}.") is the right class for holding such a test, which is not very related to connectTCP. ciao thanks ste
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Il 31/10/2010 18:36, Stefano Debenedetti ha scritto:
err, it seems to fix that issue but it introduces another issue: File "my/code.py", line 759, in loseConnection proto.transport.unregisterProducer() File "/home/lala/lib/python/twisted/internet/abstract.py", line 330, in unregisterProducer self.startWriting() File "/home/lala/lib/python/twisted/internet/abstract.py", line 290, in startWriting self.reactor.addWriter(self) File "/home/lala/lib/python/twisted/internet/epollreactor.py", line 107, in addWriter self._add(writer, self._writes, self._reads, self._selectables, _epoll.OUT, _epoll.IN) File "/home/lala/lib/python/twisted/internet/epollreactor.py", line 88, in _add self._poller._control(cmd, fd, flags) File "_epoll.pyx", line 125, in _epoll.epoll._control I just saw this appearing once in the logs of my app after I applied the patch I suggested: http://twistedmatrix.com/trac/attachment/ticket/4719/avoid_hang_in_CLOSE_WAI... I'll try to replicate and to figure out what does FileDescriptor.unregisterProducer need to do instead of the probably too dumb lines I added there: if self.disconnecting: self.startWriting() ciao ste
![](https://secure.gravatar.com/avatar/be6ed66fb77e6c5bc9ae2febc9483d41.jpg?s=120&d=mm&r=g)
Il 01/11/2010 15:34, Stefano Debenedetti ha scritto:
Alright, I added a test case that replicates the new error introduced by the above change and included it in a new proposed fix: http://twistedmatrix.com/trac/attachment/ticket/4719/avoid_hang_in_CLOSE_WAI... if self.connected and self.disconnecting: self.startWriting() This new patch now passes both new unit tests. Sorry about the noise, if I find any more issues about this I'll post them to the ticket page instead of polluting the mailing list with replies to myself. ciao ste
participants (5)
-
exarkun@twistedmatrix.com
-
Glyph Lefkowitz
-
Itamar Turner-Trauring
-
Stefano Debenedetti
-
velociraptor Genjix