From 0b07f02e19e5ed1eba0e74c3863def9f849b5f9a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Aug 2018 13:39:49 +0100 Subject: Make sure that we close db connections opened during init We should explicitly close any db connections we open, because failing to do so can block other transactions as per https://github.com/matrix-org/synapse/issues/3682. Let's also try to factor out some of the boilerplate by having server classes define their datastore class rather than duplicating the whole of `setup`. --- synapse/server.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'synapse/server.py') diff --git a/synapse/server.py b/synapse/server.py index a6fbc6ec0c..802d679cde 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -81,7 +81,6 @@ from synapse.server_notices.server_notices_manager import ServerNoticesManager from synapse.server_notices.server_notices_sender import ServerNoticesSender from synapse.server_notices.worker_server_notices_sender import WorkerServerNoticesSender from synapse.state import StateHandler, StateResolutionHandler -from synapse.storage import DataStore from synapse.streams.events import EventSources from synapse.util import Clock from synapse.util.distributor import Distributor @@ -172,6 +171,11 @@ class HomeServer(object): 'room_context_handler', ] + # This is overridden in derived application classes + # (such as synapse.app.homeserver.SynapseHomeServer) and gives the class to be + # instantiated during setup() for future return by get_datastore() + DATASTORE_CLASS = None + def __init__(self, hostname, reactor=None, **kwargs): """ Args: @@ -188,13 +192,20 @@ class HomeServer(object): self.distributor = Distributor() self.ratelimiter = Ratelimiter() + self.datastore = None + # Other kwargs are explicit dependencies for depname in kwargs: setattr(self, depname, kwargs[depname]) def setup(self): logger.info("Setting up.") - self.datastore = DataStore(self.get_db_conn(), self) + if self.DATASTORE_CLASS is None: + raise RuntimeError("%s does not define a DATASTORE_CLASS" % ( + self.__class__.__name__, + )) + with self.get_db_conn() as conn: + self.datastore = self.DATASTORE_CLASS(conn, self) logger.info("Finished setting up.") def get_reactor(self): -- cgit 1.5.1 From 32eb1dedd294831f2bafc79a24dae91eee319563 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Aug 2018 17:10:43 +0100 Subject: use abc.abstractproperty This gives clearer messages when someone gets it wrong --- synapse/server.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'synapse/server.py') diff --git a/synapse/server.py b/synapse/server.py index 802d679cde..938a05f9dc 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -19,6 +19,7 @@ # partial one for unit test mocking. # Imports required for the default HomeServer() implementation +import abc import logging from twisted.enterprise import adbapi @@ -110,6 +111,8 @@ class HomeServer(object): config (synapse.config.homeserver.HomeserverConfig): """ + __metaclass__ = abc.ABCMeta + DEPENDENCIES = [ 'http_client', 'db_pool', @@ -174,7 +177,7 @@ class HomeServer(object): # This is overridden in derived application classes # (such as synapse.app.homeserver.SynapseHomeServer) and gives the class to be # instantiated during setup() for future return by get_datastore() - DATASTORE_CLASS = None + DATASTORE_CLASS = abc.abstractproperty() def __init__(self, hostname, reactor=None, **kwargs): """ @@ -200,10 +203,6 @@ class HomeServer(object): def setup(self): logger.info("Setting up.") - if self.DATASTORE_CLASS is None: - raise RuntimeError("%s does not define a DATASTORE_CLASS" % ( - self.__class__.__name__, - )) with self.get_db_conn() as conn: self.datastore = self.DATASTORE_CLASS(conn, self) logger.info("Finished setting up.") -- cgit 1.5.1