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

10177: Sessions: use the site reactor, not the global one #1587

Merged
merged 5 commits into from May 19, 2021
Merged
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
17 changes: 9 additions & 8 deletions src/twisted/web/http.py
Expand Up @@ -3099,8 +3099,8 @@ class HTTPFactory(protocol.ServerFactory):
support writing to L{twisted.python.log} which, unfortunately, works
with native strings.

@ivar _reactor: An L{IReactorTime} provider used to compute logging
timestamps.
@ivar reactor: An L{IReactorTime} provider used to manage connection
timeouts and compute logging timestamps.
"""

# We need to ignore the mypy error here, because
Expand Down Expand Up @@ -3130,12 +3130,13 @@ def __init__(
the access log. L{combinedLogFormatter} when C{None} is passed.
@type logFormatter: L{IAccessLogFormatter} provider

@param reactor: A L{IReactorTime} provider used to manage connection
timeouts and compute logging timestamps.
@param reactor: An L{IReactorTime} provider used to manage connection
timeouts and compute logging timestamps. Defaults to the global
reactor.
"""
if not reactor:
from twisted.internet import reactor
self._reactor = reactor
self.reactor = reactor

if logPath is not None:
logPath = os.path.abspath(logPath)
Expand All @@ -3153,8 +3154,8 @@ def _updateLogDateTime(self):
"""
Update log datetime periodically, so we aren't always recalculating it.
"""
self._logDateTime = datetimeToLogString(self._reactor.seconds())
self._logDateTimeCall = self._reactor.callLater(1, self._updateLogDateTime)
self._logDateTime = datetimeToLogString(self.reactor.seconds())
self._logDateTimeCall = self.reactor.callLater(1, self._updateLogDateTime)

def buildProtocol(self, addr):
p = protocol.ServerFactory.buildProtocol(self, addr)
Expand All @@ -3164,7 +3165,7 @@ def buildProtocol(self, addr):
# ideally be resolved by passing the reactor more generally to the
# HTTPChannel, but that won't work for the TimeoutMixin until we fix
# https://twistedmatrix.com/trac/ticket/8488
p.callLater = self._reactor.callLater
p.callLater = self.reactor.callLater

# timeOut needs to be on the Protocol instance cause
# TimeoutMixin expects it there
Expand Down
1 change: 1 addition & 0 deletions src/twisted/web/newsfragments/10177.bugfix
@@ -0,0 +1 @@
Calling twisted.web.server.Site now registers its expiration timeout using the reactor associated with its twisted.web.server.Site. Site now a reactor attribute via its superclass, twisted.web.http.HTTPFactory.
41 changes: 33 additions & 8 deletions src/twisted/web/server.py
Expand Up @@ -686,12 +686,23 @@ class Session(components.Componentized):
This utility class contains no functionality, but is used to
represent a session.

@ivar site: The L{Site} that generated the session.
@type site: L{Site}

@ivar uid: A unique identifier for the session.
@type uid: L{bytes}

@ivar _reactor: An object providing L{IReactorTime} to use for scheduling
expiration.
@ivar sessionTimeout: timeout of a session, in seconds.

@ivar sessionTimeout: Time after last modification the session will expire,
in seconds.
@type sessionTimeout: L{float}

@ivar lastModified: Time the C{touch()} method was last called (or time the
session was created). A UNIX timestamp as returned by
L{IReactorTime.seconds()}.
@type lastModified L{float}
"""

sessionTimeout = 900
Expand All @@ -701,11 +712,14 @@ class Session(components.Componentized):
def __init__(self, site, uid, reactor=None):
"""
Initialize a session with a unique ID for that session.

@param reactor: L{IReactorTime} used to schedule expiration of the
session. If C{None}, the reactor associated with I{site} is used.
"""
components.Componentized.__init__(self)
super().__init__()

if reactor is None:
from twisted.internet import reactor
reactor = site.reactor
self._reactor = reactor

self.site = site
Expand Down Expand Up @@ -743,7 +757,7 @@ def expire(self):

def touch(self):
"""
Notify session modification.
Mark the session as modified, which resets expiration timer.
"""
self.lastModified = self._reactor.seconds()
if self._expireCall is not None:
Expand All @@ -758,13 +772,24 @@ class Site(http.HTTPFactory):
"""
A web site: manage log, sessions, and resources.

@ivar counter: increment value used for generating unique sessions ID.
@ivar requestFactory: A factory which is called with (channel)
and creates L{Request} instances. Default to L{Request}.

@ivar displayTracebacks: If set, unhandled exceptions raised during
rendering are returned to the client as HTML. Default to C{False}.

@ivar sessionFactory: factory for sessions objects. Default to L{Session}.
@ivar sessionCheckTime: Deprecated. See L{Session.sessionTimeout} instead.

@ivar sessions: Mapping of session IDs to objects returned by
C{sessionFactory}.
@type sessions: L{dict} mapping L{bytes} to L{Session} given the default
C{sessionFactory}

@ivar counter: The number of sessions that have been generated.
@type counter: L{int}

@ivar sessionCheckTime: Deprecated and unused. See
L{Session.sessionTimeout} instead.
"""

counter = 0
Expand All @@ -785,7 +810,7 @@ def __init__(self, resource, requestFactory=None, *args, **kwargs):

@see: L{twisted.web.http.HTTPFactory.__init__}
"""
http.HTTPFactory.__init__(self, *args, **kwargs)
super().__init__(*args, **kwargs)
self.sessions = {}
self.resource = resource
if requestFactory is not None:
Expand Down Expand Up @@ -832,7 +857,7 @@ def buildProtocol(self, addr):
"""
Generate a channel attached to this site.
"""
channel = http.HTTPFactory.buildProtocol(self, addr)
channel = super().buildProtocol(addr)
channel.requestFactory = self.requestFactory
channel.site = self
return channel
Expand Down
3 changes: 2 additions & 1 deletion src/twisted/web/test/requesthelper.py
Expand Up @@ -18,6 +18,7 @@
from twisted.internet.defer import Deferred
from twisted.internet.address import IPv4Address, IPv6Address
from twisted.internet.interfaces import ISSLTransport, IAddress
from twisted.internet.task import Clock

from twisted.trial import unittest

Expand Down Expand Up @@ -233,7 +234,7 @@ def __init__(self, postpath, session=None, client=None):
self.postpath = postpath
self.prepath = []
self.session = None
self.protoSession = session or Session(0, self)
self.protoSession = session or Session(site=None, uid=b"0", reactor=Clock())
self.args = {}
self.requestHeaders = Headers()
self.responseHeaders = Headers()
Expand Down
3 changes: 1 addition & 2 deletions src/twisted/web/test/test_cgi.py
Expand Up @@ -376,8 +376,7 @@ def test_urlParameters(self):
"""
cgiFilename = self.writeCGI(URL_PARAMETER_CGI)
portnum = self.startServer(cgiFilename)
url = "http://localhost:%d/cgi?param=1234" % (portnum,)
url = url.encode("ascii")
url = b"http://localhost:%d/cgi?param=1234" % (portnum,)
agent = client.Agent(reactor)
d = agent.request(b"GET", url)
d.addCallback(client.readBody)
Expand Down
34 changes: 23 additions & 11 deletions src/twisted/web/test/test_web.py
Expand Up @@ -15,7 +15,7 @@
from twisted.python import reflect, failure
from twisted.python.filepath import FilePath
from twisted.trial import unittest
from twisted.internet import reactor, interfaces
from twisted.internet import interfaces
from twisted.internet.address import IPv4Address, IPv6Address
from twisted.internet.task import Clock
from twisted.web import server, resource
Expand Down Expand Up @@ -214,17 +214,29 @@ def setUp(self):
"""
self.clock = Clock()
self.uid = b"unique"
self.site = server.Site(resource.Resource())
self.session = server.Session(self.site, self.uid, self.clock)
self.site = server.Site(resource.Resource(), reactor=self.clock)
self.session = server.Session(self.site, self.uid)
self.site.sessions[self.uid] = self.session

def test_defaultReactor(self):
"""
If not value is passed to L{server.Session.__init__}, the global
reactor is used.
If no value is passed to L{server.Session.__init__}, the reactor
associated with the site is used.
"""
session = server.Session(server.Site(resource.Resource()), b"123")
self.assertIdentical(session._reactor, reactor)
site = server.Site(resource.Resource(), reactor=Clock())
session = server.Session(site, b"123")
self.assertIdentical(session._reactor, site.reactor)

def test_explicitReactor(self):
"""
L{Session} accepts the reactor to use as a parameter.
"""
site = server.Site(resource.Resource())
otherReactor = Clock()

session = server.Session(site, b"123", reactor=otherReactor)

self.assertIdentical(session._reactor, otherReactor)

def test_startCheckingExpiration(self):
"""
Expand Down Expand Up @@ -761,7 +773,7 @@ def test_retrieveExistingSession(self):
request = server.Request(d, 1)
request.site = site
request.sitepath = []
mySession = server.Session(b"special-id", site)
mySession = server.Session(site, b"special-id")
site.sessions[mySession.uid] = mySession
request.received_cookies[b"TWISTED_SESSION"] = mySession.uid
self.assertIs(request.getSession(), mySession)
Expand Down Expand Up @@ -1827,11 +1839,11 @@ class ExplicitHTTPFactoryReactor(unittest.TestCase):
def test_explicitReactor(self):
"""
L{http.HTTPFactory.__init__} accepts a reactor argument which is set on
L{http.HTTPFactory._reactor}.
L{http.HTTPFactory.reactor}.
"""
reactor = "I am a reactor!"
factory = http.HTTPFactory(reactor=reactor)
self.assertIs(factory._reactor, reactor)
self.assertIs(factory.reactor, reactor)

def test_defaultReactor(self):
"""
Expand All @@ -1841,4 +1853,4 @@ def test_defaultReactor(self):
from twisted.internet import reactor

factory = http.HTTPFactory()
self.assertIs(factory._reactor, reactor)
self.assertIs(factory.reactor, reactor)
2 changes: 0 additions & 2 deletions tox.ini
Expand Up @@ -19,7 +19,6 @@
minversion=3.21.4
requires=
virtualenv>=20.4.2
tox-wheel>=0.6.0
skip_missing_interpreters=True
envlist=lint, mypy,
apidocs, narrativedocs, newsfragment,
Expand All @@ -35,7 +34,6 @@ sources = setup.py src/ docs/conch/examples docs/mail/examples docs/names/exampl
; docs/core/examples

[testenv]
wheel = True
;; dependencies managed by extras in setup.cfg
extras =
; The "nodeps" build still depends on PyHamcrest.
Expand Down