From 1f3752cb108d85c850113904d6b1308fbc1a53c1 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Mon, 30 Mar 2015 18:22:07 -0700 Subject: [PATCH] Address concurrent mutation of sqlalchemy backend In order to prevent a thread from closing a backend while another thread is getting a connection (which can result in an engine being created) stop this kind of concurrent mutation by creating a engine (if it was not user provided) in the constructor. In the close the engine dispose is called (which will according to the docs just create a new pool anyway) so there is no need to recreate the full engine object from its same configuration again. Change-Id: Id1fa3001b3ebbe76bbcdb08ed4add6a9e16ea96b --- .../persistence/backends/impl_sqlalchemy.py | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/taskflow/persistence/backends/impl_sqlalchemy.py b/taskflow/persistence/backends/impl_sqlalchemy.py index 483dd445..a06850df 100644 --- a/taskflow/persistence/backends/impl_sqlalchemy.py +++ b/taskflow/persistence/backends/impl_sqlalchemy.py @@ -237,14 +237,15 @@ class SQLAlchemyBackend(base.Backend): self._engine = engine self._owns_engine = False else: - self._engine = None + self._engine = self._create_engine(self._conf) self._owns_engine = True self._validated = False - def _create_engine(self): + @staticmethod + def _create_engine(conf): # NOTE(harlowja): copy the internal one so that we don't modify it via # all the popping that will happen below. - conf = copy.deepcopy(self._conf) + conf = copy.deepcopy(conf) engine_args = { 'echo': _as_bool(conf.pop('echo', False)), 'convert_unicode': _as_bool(conf.pop('convert_unicode', True)), @@ -307,8 +308,6 @@ class SQLAlchemyBackend(base.Backend): @property def engine(self): - if self._engine is None: - self._engine = self._create_engine() return self._engine def get_connection(self): @@ -323,15 +322,11 @@ class SQLAlchemyBackend(base.Backend): return conn def close(self): - if self._engine is not None and self._owns_engine: - # NOTE(harlowja): Only dispose of the engine and clear it from - # our local state if we actually own the engine in the first - # place. If the user passed in their own engine we should not - # be disposing it on their behalf (and we shouldn't be clearing - # our local engine either, since then we would just recreate a - # new engine if the engine property is accessed). + # NOTE(harlowja): Only dispose of the engine if we actually own the + # engine in the first place. If the user passed in their own engine + # we should not be disposing it on their behalf... + if self._owns_engine: self._engine.dispose() - self._engine = None self._validated = False