Change in vdsm[ovirt-3.4]: threadLocal: Cleanup thread local storage
nsoffer at redhat.com
nsoffer at redhat.com
Wed Feb 26 22:50:17 UTC 2014
Hello Adam Litke, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/25123
to review the following change.
Change subject: threadLocal: Cleanup thread local storage
......................................................................
threadLocal: Cleanup thread local storage
We keep some data in thread local storage, but never clean up. This can
lead to using stale data when handling many requests in the same tread,
or to temporary leak, when a long running thread holds a reference to a
finished task, preventing garbage collection of anything referenced from
the task.
This patch ensures that anything we set thread local storage is cleaned
up when not needed.
Change-Id: I97b110db1c5f94e8561b4ba15a3c27b43043ed6f
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24309
Reviewed-by: Adam Litke <alitke at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/BindingXMLRPC.py
M vdsm/storage/dispatcher.py
M vdsm/storage/storage_mailbox.py
M vdsm/storage/task.py
4 files changed, 25 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/25123/1
diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py
index 9b56ea1..bafc7c7 100644
--- a/vdsm/BindingXMLRPC.py
+++ b/vdsm/BindingXMLRPC.py
@@ -212,6 +212,12 @@
threadLocal.flowID = self.headers.get(HTTP_HEADER_FLOWID)
return r
+ def finish(self):
+ basehandler.finish(self)
+ threadLocal.client = None
+ threadLocal.server = None
+ threadLocal.flowID = None
+
if self.enableSSL:
KEYFILE, CERTFILE, CACERT = self._getKeyCertFilenames()
server = SecureXMLRPCServer.SecureThreadedXMLRPCServer(
diff --git a/vdsm/storage/dispatcher.py b/vdsm/storage/dispatcher.py
index cbeb23c..da838e8 100644
--- a/vdsm/storage/dispatcher.py
+++ b/vdsm/storage/dispatcher.py
@@ -23,7 +23,6 @@
import task
import storage_exception as se
-from threadLocal import vars
_EXPORTED_ATTRIBUTE = "__dispatcher_exported__"
@@ -56,7 +55,6 @@
def run(self, *args, **kwargs):
try:
ctask = task.Task(id=None, name=self.name)
- vars.task = ctask
try:
response = self.STATUS_OK.copy()
result = ctask.prepare(self.func, *args, **kwargs)
diff --git a/vdsm/storage/storage_mailbox.py b/vdsm/storage/storage_mailbox.py
index d56a2d7..4f54066 100644
--- a/vdsm/storage/storage_mailbox.py
+++ b/vdsm/storage/storage_mailbox.py
@@ -33,7 +33,6 @@
import sd
import misc
import task
-from threadLocal import vars
from threadPool import ThreadPool
from storage_exception import InvalidParameterException
from vdsm import constants
@@ -77,7 +76,6 @@
cmd = args
args = None
ctask = task.Task(id=None, name=cmd)
- vars.task = ctask
ctask.prepare(cmd, *args)
diff --git a/vdsm/storage/task.py b/vdsm/storage/task.py
index 4eff5c1..88ab5fa 100644
--- a/vdsm/storage/task.py
+++ b/vdsm/storage/task.py
@@ -49,6 +49,7 @@
import logging
import threading
import types
+from functools import wraps
import storage_exception as se
import uuid
@@ -88,6 +89,21 @@
def _eq_decode(s):
return s.replace(KEY_SEPARATOR_ENCODED, KEY_SEPARATOR)
+
+
+def threadlocal_task(m):
+ """
+ Decorator that set the task object in thread local storage task attribute
+ while the decorated method is running.
+ """
+ @wraps(m)
+ def wrapper(self, *a, **kw):
+ vars.task = self
+ try:
+ return m(self, *a, **kw)
+ finally:
+ vars.task = None
+ return wrapper
class State:
@@ -1133,6 +1149,7 @@
t._load(store, ext)
return t
+ @threadlocal_task
def prepare(self, func, *args, **kwargs):
message = self.error
try:
@@ -1172,9 +1189,9 @@
finally:
self._decref()
+ @threadlocal_task
def commit(self, args=None):
self.log.debug("committing task: %s", self.id)
- vars.task = self
try:
self._incref()
except se.TaskAborted:
@@ -1210,13 +1227,13 @@
finally:
self._decref(force)
+ @threadlocal_task
def recover(self, args=None):
''' Do not call this function while the task is actually running. this
method should only be used to recover tasks state after
(vdsmd) restart.
'''
self.log.debug('(recover): recovering: state %s', self.state)
- vars.task = self
try:
self._incref(force=True)
except se.TaskAborted:
--
To view, visit http://gerrit.ovirt.org/25123
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I97b110db1c5f94e8561b4ba15a3c27b43043ed6f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
More information about the vdsm-patches
mailing list