Opened 4 years ago

Closed 3 years ago

Last modified 2 years ago

#9446 defect closed fixed (fixed)

iocp reactor is duplicating/losing outgoing data

Reported by: asodeur Owned by: Glyph
Priority: highest Milestone:
Component: core Keywords: iocp
Cc: Branch: 9446-asodeur-issue9446
branch-diff, diff-cov, branch-cov, buildbot
Author:

Description (last modified by asodeur)

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 (1)

0673-updated-test-to-check-iocp-reactor-race-condition.patch (2.0 KB) - added by asodeur 4 years ago.
Patch to LargeBufferTests for iocp reactor race conditions

Download all attachments as: .zip

Change History (11)

Changed 4 years ago by asodeur

Patch to LargeBufferTests for iocp reactor race conditions

comment:1 Changed 3 years ago by asodeur

Description: modified (diff)
Keywords: review added

comment:2 Changed 3 years ago by teratorn

Summary: iocp reactor is duplicating/loosing outgoing dataiocp reactor is duplicating/losing outgoing data

comment:3 Changed 3 years ago by asodeur

Description: modified (diff)
Priority: highhighest

comment:4 Changed 3 years ago by asodeur

Description: modified (diff)
Owner: set 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

comment:5 Changed 3 years ago by Wim Lewis

Branch: 9446-asodeur-issue9446

comment:6 Changed 3 years ago by Glyph

Owner: hawkowl deleted

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 :)

comment:7 Changed 3 years ago by Glyph

Wim Lewis pointed out that this may be a duplicate of a much earlier bug, https://twistedmatrix.com/trac/ticket/3525

comment:8 Changed 3 years ago by Glyph

Keywords: review removed
Owner: set 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.

comment:9 Changed 3 years ago by Glyph <glyph@…>

Resolution: fixed
Status: newclosed

In 92c1353b:

Error: Processor CommitTicketReference failed
 does not appear to be a Git repository. See the log for more information.

comment:10 Changed 2 years ago by Glyph

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

Note: See TracTickets for help on using tickets.