iocp reactor is duplicating/losing outgoing data

[twisted-trac]

	@asodeur reported
Trac ID	trac#9446
Type	defect
Created	2018-05-14 08:01:49Z
Branch	https://github.com/twisted/twisted/tree/9446-asodeur-issue9446

Writing new data to a saturated TCP connection can cause outgoing data to be duplicated. IMHO the problem is iocpreactor.abstract.FileHandle.doWrite can get called again before ._handleWrite got the chance to update the bytes already written.

The new test_tcp.LargeBufferTests writes the last byte in a separate .write(). Sadly, behaviour is not fully deterministic. Most runs fail on iocp reactor with too many bytes received, some fail with not enough bytes received (have not understood this to date), and very few run ok.Everything works fine on the select reactor.

This PR 1025 should be a fix.

Attachments:

• 0673-updated-test-to-check-iocp-reactor-race-condition.patch (2001 bytes) - added by asodeur on 2018-05-14 08:04:57Z - Patch to LargeBufferTests for iocp reactor race conditions

Searchable metadata

trac-id__9446 9446
type__defect defect
reporter__asodeur asodeur
priority__highest highest
milestone__None None
branch__9446_asodeur_issue9446 9446-asodeur-issue9446
branch_author__ 
status__closed closed
resolution__fixed fixed
component__core core
keywords__iocp iocp
time__1526284909878457 1526284909878457
changetime__1568870753410137 1568870753410137
version__None None
owner__glyph glyph

[twisted-trac]

	@asodeur set owner to @hawkowl

@hawkowl, sorry to assign to you directly but since you are working on py3.7 support for the iocp reactor you are hopefully the right person. I am not 100% the proposed solution in the PR is how it should be done but the new test_tcp.LargeBufferTests demonstrates the issue with the current iocp reactor.

Andreas

[twisted-trac]

	@glyph removed owner

Un-assigning because I don't believe this requires a specific reviewer. Please feel free to entice hawkie to review it by other means though :)

[twisted-trac]

	@glyph commented

Wim Lewis pointed out that this may be a duplicate of a much earlier bug, #3525

[twisted-trac]

	@glyph set owner to @glyph

It looks like the author has received several comments from Adi and addressed all of them.

I'll be landing this once tests have finished running.

[twisted-trac]

	@glyph set status to closed

In changeset 92c1353

#!CommitTicketReference repository="" revision="92c1353bf61c36f821fab959fe64eea252be8a5b"
Merge pull request #1025 from asodeur/issue9446

Author: asodeur

Reviewer: adiroiban, glyph

Fixes: ticket:9446

The IOCP reactor no longer duplicates/loses outgoing data when .write() is called in rapid succession with large payloads.

[twisted-trac]

	@glyph commented

I've closed #9353 and #3525 because I'm pretty sure these were all the same issue.