Opened 6 years ago

Closed 4 years ago

#5634 enhancement closed fixed (fixed)

ReactorBuilder.runReactor should cancel the timeout it creates if it wasn't triggered

Reported by: Itamar Turner-Trauring Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/reactorbuilder-runreactor-timeout-5634
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

twisted.internet.test.test_tcp.AbortConnectionMixin needs to manually deal with the timeout added by ReactorBuilder.runReactor. It would be better if runReactor cleaned up after itself and cancelled the timeout it scheduled if it exists before the timeout is reached.

Attachments (1)

5634.patch (1.3 KB) - added by Saurabh 4 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by Saurabh

Owner: set to Saurabh

Changed 4 years ago by Saurabh

Attachment: 5634.patch added

comment:2 Changed 4 years ago by Saurabh

Keywords: review added
Owner: Saurabh deleted

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

Author: exarkun
Branch: branches/reactorbuilder-runreactor-timeout-5634

(In [40331]) Branching to 'reactorbuilder-runreactor-timeout-5634'

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

(In [40332]) apply 5634.patch

refs #5634

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

Keywords: review removed
Owner: set to Jean-Paul Calderone

Thanks.

It looks like the new if could just be an else on the if that's there already. Also, the change needs a news fragment - but just a misc, I expect.

I'll make these two minor changes and apply. Thanks again.

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

Oh. Fortunately before I merged I also noticed that the change to the tests is wrong. The assertion near the comment should be changed - not deleted.

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

(In [40335]) Restore the assertion about delayed calls in the reactor (or lack thereof).

refs #5634

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

Resolution: fixed
Status: newclosed

(In [40340]) Merge reactorbuilder-runreactor-timeout-5634

Author: Saurabh Reviewer: exarkun Fixes: #5634

Cancel the timeout set up by ReactorBuilder.runReactor and remove the work-around in a TCP unit test that previously accounted for this extra delayed call left lying around.

Note: See TracTickets for help on using tickets.