id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
11193	WSGI handler not properly handling lock	Phillip Sitbon	nobody	"For this code in wsgi.py, seen in the latest release and the [source:django/trunk/django/core/handlers/wsgi.py@9996#L226 latest repository revision]:

{{{
#!python
        if self._request_middleware is None:
            self.initLock.acquire()
            # Check that middleware is still uninitialised.
            if self._request_middleware is None:
                self.load_middleware()
            self.initLock.release()
}}}

I noticed the issue of thread safety in this handler came up in #10470, but this code is still not thread safe because it it can be put into a deadlock state. It will deadlock after any of the middleware loading fails- for example, when the module pointed to by DJANGO_SETTINGS_MODULE isn't found (which is how I came to notice it). After the failure, initLock is held and so this can deadlock on initLock.acquire().

The fix is very simple, and should '''absolutely''' be done anywhere locking is done:

{{{
#!python
        if self._request_middleware is None:
            self.initLock.acquire()
            try:
                # Check that middleware is still uninitialised.
                if self._request_middleware is None:
                    self.load_middleware()
            finally:
                self.initLock.release()
}}}

Of course using with keyword is an option as well. Also, I'm not sure how you would handle an indeterminate state between initialized and not initialized (e.g. some middleware loaded), but it might be a good idea to set self._request_middleware back to None before releasing the lock in case of an exception:


{{{
#!python
        if self._request_middleware is None:
            self.initLock.acquire()
            try:
                # Check that middleware is still uninitialised.
                if self._request_middleware is None:
                    self.load_middleware()
            except:
                # possibly unload whatever middleware didn't fail here
                self._request_middleware = None
                raise
            finally:
                self.initLock.release()
}}}
"		closed	Core (Other)	1.0		fixed	threads, locks		Accepted	0	0	0	0	0	0