10

I have a large python application which is running on a Django service. I need to turn off permission tests for certain operations so I created this context manager:

class OverrideTests(object):

    def __init__(self):
        self.override = 0

    def __enter__(self):
        self.override += 1

    # noinspection PyUnusedLocal
    def __exit__(self, exc_type, exc_val, exc_tb):
        self.override -= 1
        assert not self.override < 0

    @property
    def overriding(self):
        return self.override > 0

override_tests = OverrideTests()

Various parts of the application can then overide the tests using the context manager:

with override_tests:
    do stuff
    ...

Within the do stuff, the above context manager may be used multiple times in different functions. The use of the counter keeps this under control and it seems to work fine... until threads get involved.

Once there are threads involved, the global context manager gets re-used and as a result, tests may be incorrectly over-ridden.

Here is a simple test case - this works fine if the thread.start_new_thread(do_id, ()) line is replaced with a simple do_it but fails spectacularly as shown:

def stat(k, expected):
    x = '.' if override_tests.overriding == expected else '*'
    sys.stdout.write('{0}{1}'.format(k, x))


def do_it_inner():
    with override_tests:
        stat(2, True)
    stat(3, True)  # outer with context makes this true


def do_it():
    with override_tests:
        stat(1, True)
        do_it_inner()
    stat(4, False)


def do_it_lots(ntimes=10):
    for i in range(ntimes):
        thread.start_new_thread(do_it, ())

How can I make this context manager thread safe so that in each Python thread, it is consistently used even though it is re-entrant?

Paul Whipp
  • 16,028
  • 4
  • 42
  • 54
  • 2
    Have you tried not creating a single context manager, and instead using `with OverrideTests()`, creating a separate instance for every usage? – BrenBarn Oct 29 '15 at 06:47
  • The test code accesses the 'global value' on the context manager singleton in order to know whether or not to apply the test. This is not in the immediate context of the with statement. There is `if override_tests.overriding: return True` in the test methods so that they are overriden. I can't do that if there are multiple context manager instances. – Paul Whipp Oct 29 '15 at 07:27
  • 2
    Anything that relies on mutating global state is going to run into problems with threads. It sounds like you might need to rethink the whole structure here. Having functions that alter their behavior by checking that kind of global flag is not very robust. You could instead, for instance, put all the test functions into a class, so that each instance stores its own override status, and then create a new instance of that class per thread. – BrenBarn Oct 29 '15 at 07:54
  • Pooling the tests into a single class is not possible. I was looking at threading.local() but I've not managed to make that work either yet. – Paul Whipp Oct 29 '15 at 08:17

1 Answers1

8

Here is a way that seems to work: make your OverrideTests class a subclass of threading.local. For safety, you should then call the superclass __init__ in your __init__ (although it seems to work even if you don't):

class OverrideTests(threading.local):

    def __init__(self):
        super(OverrideTests, self).__init__()
        self.override = 0

    # rest of class same as before

override_tests = OverrideTests()

Then:

>>> do_it_lots()
1.1.1.2.2.1.1.1.1.1.1.3.3.2.2.2.2.2.2.4.4.3.1.3.3.3.3.4.3.2.4.4.2.4.3.4.4.4.3.4.

However, I wouldn't put money on this not failing in some kind of corner case, especially if your real application is more complex than the example you showed here. Ultimately, you really should rethink your design. In your question, you are focusing on how to "make the context-manager threadsafe". But the real problem is not just with your context manager but with your function (stat in your example). stat is relying on global state (the global override_tests), which is inherently fragile in a threaded environment.

BrenBarn
  • 242,874
  • 37
  • 412
  • 384
  • OK - Looks like your solution is working so far. I've submitted the change to our QA server and will approve this answer if we don't see a repro there in the next 24 hours. – Paul Whipp Nov 02 '15 at 07:14
  • There will always be times when reliance upon some global state is required and where passing such state entirely through functional parametrization is impractical. My real world example was actually kept pretty much as simple as this example and it appears to be working well. – Paul Whipp Nov 02 '15 at 19:33
  • This has been running for some time now and it seems to have fully addressed the issue. – Paul Whipp Nov 09 '15 at 04:26