Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

let's try invalidating reactors and see if that gives us a hint #12085

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 15 additions & 13 deletions src/twisted/internet/cfreactor.py
Expand Up @@ -197,11 +197,11 @@
# reading/writing state flags to determine whether we
# should actually attempt a doRead/doWrite first. -glyph
if isRead:
if rw[_READ]:
why = readWriteDescriptor.doRead()
assert rw[_READ], "read callback when twisted not reading"
why = readWriteDescriptor.doRead()

Check warning on line 201 in src/twisted/internet/cfreactor.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/internet/cfreactor.py#L200-L201

Added lines #L200 - L201 were not covered by tests
else:
if rw[_WRITE]:
why = readWriteDescriptor.doWrite()
assert rw[_WRITE], "write callback when twisted not writing"
why = readWriteDescriptor.doWrite()

Check warning on line 204 in src/twisted/internet/cfreactor.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/internet/cfreactor.py#L203-L204

Added lines #L203 - L204 were not covered by tests
except BaseException:
why = sys.exc_info()[1]
log.err()
Expand All @@ -225,10 +225,12 @@
@param flag: the flag to register for callbacks on, either
C{kCFSocketReadCallBack} or C{kCFSocketWriteCallBack}
"""
assert isinstance(fd, int), f"only ints allowed, not {fd!r}"

Check warning on line 228 in src/twisted/internet/cfreactor.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/internet/cfreactor.py#L228

Added line #L228 was not covered by tests
if fd == -1:
raise RuntimeError("Invalid file descriptor.")
if fd in self._fdmap:
src, cfs, gotdescr, rw = self._fdmap[fd]
assert gotdescr is descr, f"mismatch between {descr} and {gotdescr}"

Check warning on line 233 in src/twisted/internet/cfreactor.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/internet/cfreactor.py#L233

Added line #L233 was not covered by tests
# do I need to verify that it's the same descr?
else:
ctx = []
Expand Down Expand Up @@ -295,18 +297,18 @@

@param flag: C{kCFSocketWriteCallBack} C{kCFSocketReadCallBack}
"""
if id(descr) not in self._idmap:
descrid = id(descr)

Check warning on line 300 in src/twisted/internet/cfreactor.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/internet/cfreactor.py#L300

Added line #L300 was not covered by tests
if descrid not in self._idmap:
return
if fd == -1:
# need to deal with it in this case, I think.
realfd = self._idmap[id(descr)]
else:
realfd = fd
src, cfs, descr, rw = self._fdmap[realfd]
realfd = self._idmap[descrid]
src, cfs, gotdescr, rw = self._fdmap[realfd]
assert (

Check warning on line 305 in src/twisted/internet/cfreactor.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/internet/cfreactor.py#L303-L305

Added lines #L303 - L305 were not covered by tests
gotdescr is descr
), f"unwatching file descriptor mismatch {descr!r} {gotdescr!r}"
CFSocketDisableCallBacks(cfs, flag)
rw[self._flag2idx(flag)] = False
if not rw[_READ] and not rw[_WRITE]:
del self._idmap[id(descr)]
if fd != realfd or (not rw[_READ] and not rw[_WRITE]):
del self._idmap[descrid]

Check warning on line 311 in src/twisted/internet/cfreactor.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/internet/cfreactor.py#L311

Added line #L311 was not covered by tests
del self._fdmap[realfd]
CFRunLoopRemoveSource(self._cfrunloop, src, kCFRunLoopCommonModes)
CFSocketInvalidate(cfs)
Expand Down
15 changes: 15 additions & 0 deletions src/twisted/internet/test/reactormixins.py
Expand Up @@ -15,6 +15,7 @@

__all__ = ["TestTimeoutError", "ReactorBuilder", "needsRunningReactor"]

import gc
import os
import signal
import time
Expand Down Expand Up @@ -106,6 +107,13 @@ def stopIfError(event):
case.addCleanup(publisher.removeObserver, stopIfError)


class InvalidatedReactor:
"""
This object has no methods and no state, and will hopefully explode if any
code attempts to interact with a reactor from a previous test.
"""


class ReactorBuilder:
"""
L{SynchronousTestCase} mixin which provides a reactor-creation API. This
Expand Down Expand Up @@ -254,6 +262,13 @@ def _unbuildReactor(self, reactor):
globalReactor.__dict__ = reactor._originalReactorDict
globalReactor.__class__ = reactor._originalReactorClass

# Some reactors (notably, CFReactor) manipulate global state if you
# interact with them; doing so across tests is inherently a potential
# state corruption. Let's make that more visible.
reactor.__class__ = InvalidatedReactor
reactor.__dict__ = {}
gc.collect()

def buildReactor(self):
"""
Create and return a reactor using C{self.reactorFactory}.
Expand Down
Empty file.