Opened 5 years ago

Closed 4 years ago

#6217 defect closed invalid (invalid)

iocpreactor events do not occur at the time they should

Reported by: BrianM Owned by:
Priority: normal Milestone:
Component: core Keywords: iocpreactor
Cc: Branch:
Author:

Description

When multiple writes are queued to a Perspective Broker connection, the success callbacks all occur at the same time. When using the default select reactor, the events occur as each write succeeds.

Attached are two scripts, pbecho.py (server) and pbechoclient.py (the client) with the select reactor (iocpreactor commented out) the 'Transfer Done' messages in the client occur when 'Transfer some data' messages appear in the server.

With the iocpreactor, the 'Transfer Done' messages all occur at once when all 4 data transfers are complete.

Attachments (4)

pbecho.py (1.6 KB) - added by BrianM 5 years ago.
pbechoclient.py (2.0 KB) - added by BrianM 5 years ago.
reactor.py.diff (1.6 KB) - added by BrianM 5 years ago.
test_iocp_sequence.py (4.5 KB) - added by BrianM 5 years ago.

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by BrianM

Attachment: pbecho.py added

Changed 5 years ago by BrianM

Attachment: pbechoclient.py added

comment:1 Changed 5 years ago by BrianM

Milestone: regular-releases
Priority: normalhigh

comment:2 Changed 5 years ago by Jean-Paul Calderone

Keywords: twisted reactor removed
Milestone: regular-releases
Priority: highnormal

The regular-releases milestone is for release automation tools.

comment:3 Changed 5 years ago by BrianM

The problem is in iocpreactor/reactor.py. The doIteration loop fetches an event, and processes the callback immediately. This can result in a self-perpetuating loop if the callback produces more I/O events. The fix is to fetch all events first, then process them. This makes it the same as the selectreactor. I have attached a unified diff made against revision #32813 that fixes the issue. So far I have not been able ot make a simple one-process unit test to show this, I will try later. It is easy to reproduce in two-process tests.

Changed 5 years ago by BrianM

Attachment: reactor.py.diff added

Changed 5 years ago by BrianM

Attachment: test_iocp_sequence.py added

comment:4 Changed 5 years ago by BrianM

Unit tes test_iocp_sequence added. It fails when using iocpreactor, passes with selectreactor, and passes with the patched iocpreactor

comment:5 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review added

Thanks for the patch! Please add "review" keyword with patches, so we know to look at them.

comment:6 Changed 4 years ago by Tom Prince

Keywords: review removed
Resolution: invalid
Status: newclosed

Thanks for your contribution. Sorry it took so long for feedback.

  1. The code is now ignoring EVENTS_PER_LOOP. If, for some reason, events are arriving faster than we can collect them, then none of them will ever be processed.
  2. Looking at the test case you included, I'm not convinced that the behavior you are testing for is behavior we want to guarantee.
    • The test fails for me with serveral reactors on linux.
    • callLater(0, ...) means call this soon, but doesn't give any guarantees relative to any other events.
  3. There are a number of coding style violations:
    1. test names should be test_somethingDoesSomething
    2. names should be camelCase or, for constants, ALL_CAPS
  4. The test should either be a ReactorBuilder test (if it should be true for all reactors), or possibly in twisted.internet.test.test_iocp.

I'm going to close this ticket, as I don't think the behavior described is a bug. Feel free to reopen, if appropriate.

Note: See TracTickets for help on using tickets.