#6217 defect closed invalid (invalid)

iocpreactor events do not occur at the time they should

Reported by: BrianMatthews Owned by:
Priority: normal Milestone:
Component: core Keywords: iocpreactor
Cc: Branch:
Author: Launchpad Bug:

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 BrianMatthews 22 months ago.
pbechoclient.py (2.0 KB) - added by BrianMatthews 22 months ago.
reactor.py.diff (1.6 KB) - added by BrianMatthews 22 months ago.
test_iocp_sequence.py (4.5 KB) - added by BrianMatthews 22 months ago.

Download all attachments as: .zip

Change History (10)

Changed 22 months ago by BrianMatthews

Changed 22 months ago by BrianMatthews

comment:1 Changed 22 months ago by BrianMatthews

  • Milestone set to regular-releases
  • Priority changed from normal to high

comment:2 Changed 22 months ago by exarkun

  • Keywords twisted reactor removed
  • Milestone regular-releases deleted
  • Priority changed from high to normal

The regular-releases milestone is for release automation tools.

comment:3 Changed 22 months ago by BrianMatthews

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 22 months ago by BrianMatthews

Changed 22 months ago by BrianMatthews

comment:4 Changed 22 months ago by BrianMatthews

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

comment:5 Changed 18 months ago by itamar

  • Keywords review added

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

comment:6 Changed 16 months ago by tom.prince

  • Keywords review removed
  • Resolution set to invalid
  • Status changed from new to closed

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.