[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Change by Irit Katriel : -- resolution: -> wont fix stage: -> resolved status: open -> closed superseder: -> Close asyncore/asynchat/smtpd issues and list them here ___ Python tracker

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Change by Serhiy Storchaka : -- type: crash -> behavior ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Pierrick Koch added the comment: Fix patch from Xavier's comment, sorry for the delay. Lib/test/test_asynchat.py passes (Ran 27 tests in 1.431s) -- Added file: http://bugs.python.org/file33898/cpython.asyncore_4.patch ___ Python tracker

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Xavier de Gaye added the comment: I think you mean: self.producer_fifo.extendleft([data, first]) Instead of: self.producer_fifo.extendleft([first, data]) No? No, I do mean: self.producer_fifo.extendleft([first, data]) See the documentation. Also: from collections

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Xavier de Gaye added the comment: After applying the last patch cpython.asyncore_3.patch (while cpython.asyncore_2.patch does not fail): == FAIL: test_simple_producer (test.test_asynchat.TestAsynchat)

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Pierrick Koch added the comment: last patch, replaced: self.producer_fifo.appendleft([data, first]) by: self.producer_fifo.extendleft([data, first]) -- Added file: http://bugs.python.org/file30612/cpython.asyncore_3.patch ___ Python tracker

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Xavier de Gaye added the comment: Attached are two test cases for this patch. test_simple_producer still fails with the new patch because it should be: self.producer_fifo.extendleft([first, data]) instead of: self.producer_fifo.appendleft([data, first]) The order of the items in the

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Andrew Stormont added the comment: I think you mean: self.producer_fifo.extendleft([data, first]) Instead of: self.producer_fifo.extendleft([first, data]) No? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17925

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Pierrick Koch added the comment: sorry for the delay, here is the updated patch, shall I add a new class in Lib/test/test_asynchat.py ? -- Added file: http://bugs.python.org/file30545/cpython.asyncore.patch ___ Python tracker rep...@bugs.python.org

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Andrew Stormont added the comment: Great. Everybody's happy now, surely? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17925 ___ ___

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Giampaolo Rodola' added the comment: patch plus self.producer_fifo.extendleft([data, first]) seems legit and I verified pyftpdlib tests pass. Last thing missing from the patch is a test case. Pierrick can you merge test_initiate_send.py into Lib/test_asynchat.py and provide a new patch?

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Xavier de Gaye added the comment: The problem is that when the fifo contains a producer and the more() method of the producer returns a non-empty bytes sequence, then the producer must be put back in the fifo first. This is what does the following change made to Pierrick patch: diff --git

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Giampaolo Rodola' added the comment: I think we shouldn't expect asynchat to be thread safe in this regard. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17925 ___

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Andrew Stormont added the comment: What about changing: self.producer_fifo.appendleft(first) self.producer_fifo.appendleft(data) To self.producer_fifo.extendleft([data, first]) Assuming deque's extendleft is actually thread safe. -- ___ Python

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Xavier de Gaye added the comment: extendleft is an extension module C function (in the _collections module) that does not release the GIL, so it is thread safe. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17925

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Andrew Stormont added the comment: Looks like a reasonable fix to me. What needs to be done to get this integrated? -- nosy: +andy_js ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17925

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Giampaolo Rodola' added the comment: After applying the patch I get these 2 failures on Linux: == FAIL: test_simple_producer (__main__.TestAsynchat) --

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Xavier de Gaye added the comment: The attached script, test_initiate_send.py, tests initiate_send with threads, causing duplicate writes and an IndexError. This is the script output using python on the default branch: $ python test_initiate_send.py --- Test: duplicate data sent ---

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

New submission from Pierrick Koch: Dear, del deque[0] is not safe, see the attached patch for the asynchat.async_chat.initiate_send method. fix the IndexError: deque index out of range of del self.producer_fifo[0] Best, Pierrick Koch -- components: Library (Lib) files:

[issue17925] asynchat.async_chat.initiate_send : del deque[0] is not safe

Changes by R. David Murray rdmur...@bitdance.com: -- nosy: +giampaolo.rodola ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17925 ___ ___