Opened 10 years ago

Closed 10 years ago

#3200 release blocker: regression closed fixed (fixed)

Update iocp reactor to manage correctly the new _stopped flag

Reported by: therve Owned by:
Priority: high Milestone: Twisted-8.1
Component: core Keywords:
Cc: Branch: branches/iocp-fix-3200
branch-diff, diff-cov, branch-cov, buildbot
Author: therve

Description

Since r23345, ReactorBase now uses a new _stopped flag to correctly manage its running state. We should update the iocp reactor to use it, because it's not working correctly anymore.

The correct workflow might have been to revert the changeset, but we concluded that it could be bypassed here.

Change History (9)

comment:1 Changed 10 years ago by therve

author: therve
Branch: branches/iocp-fix-3200

(In [23378]) Branching to 'iocp-fix-3200'

comment:2 Changed 10 years ago by therve

Keywords: review added
Owner: changed from therve to Jean-Paul Calderone

Please have a look.

comment:3 Changed 10 years ago by therve

Summary: Update iocp reactor the manage correctly the new _stopped flagUpdate iocp reactor to manage correctly the new _stopped flag

comment:4 Changed 10 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to therve

This looks pretty good.

Can you add a bit to the _SignalReactorMixin docstring? The mixin expects some methods to be implemented on the class mixing it in - eg, sigInt, sometimes sigBreak, callLater, mainLoop. It also pretty much requires that it be mixed in with ReactorBase, since it calls up to a method of that class. Maybe it would make more sense if there to be _SignalReactorBase subclassing ReactorBase and subclassed by everyone else?

comment:5 Changed 10 years ago by therve

(In [23390]) Add some doc, remove some more duplication.

Refs #3200

comment:6 Changed 10 years ago by therve

Keywords: review added
Owner: changed from therve to Jean-Paul Calderone

Okay, I updated documentation a bit. It's not awesome yet, so if you have some more idea don't hesitate. I didn't change the class hierarchy for now, because I think this is in flux so we just have to wait for something a little bit better. Please review.

comment:7 Changed 10 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to therve

Looks good to me, please merge.

comment:8 Changed 10 years ago by therve

Resolution: fixed
Status: newclosed

(In [23392]) Merge iocp-fix-3200

Author: therve Reviewer: exarkun Fixes #3200

Fix the iocpreactor management of the new _stopped attribute introduced in r23345. It removes a fair amount of duplication between iocpreactor and posixbase in the process.

comment:9 Changed 7 years ago by <automation>

Owner: therve deleted
Note: See TracTickets for help on using tickets.