bitmath icon indicating copy to clipboard operation
bitmath copied to clipboard

The format context mananger is not threadsafe

Open walrusVision opened this issue 6 years ago • 3 comments

Using the format context manager in a threaded environment introduces a race condition where the global formatter and global plural flag can be reset by one context manager after another context manager has set it, and before it has formatted a string within that context.

A solution to this is bitmath should either make sure that the context manager is threadsafe, or warn in the documentation that the context manager isn't threadsafe and that a user should use the Bitmath object's format function directly instead.

How to REPRODUCE the issue: This test case can reproduce the issue. The test is only checking format_string but a similar method could be used to test format_plural

#! /usr/bin/env python

from __future__ import print_function

import random
import queue
import threading
import time
import unittest

import bitmath

default_format = bitmath.format_string
bitmath.format_string = "{not_in_context_manager}"

THREAD_COUNT = 8

def format_string(event, err_queue):
    size = bitmath.KiB(0)
    try:
        with bitmath.format(fmt_str=default_format):
            event.wait()
            time.sleep(random.random() * 2)
            str(size)
    except Exception as e:
        err_queue.put(e)


class TestThreading(unittest.TestCase):

    def test_context_manager(self):
        event = threading.Event()
        err_queue = queue.Queue()
        threads = []
        for i in xrange(THREAD_COUNT):
            threads.append(threading.Thread(target=format_string, args=(event, err_queue)))
        for thread in threads:
            thread.start()
        time.sleep(1)
        event.set()
        for thread in threads:
            thread.join()

        error = err_queue.get()
        if error:
            raise error

if __name__ == '__main__':
    unittest.main()

And will produce the following error

======================================================================
ERROR: test_context_manager (__main__.TestThreading)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./test_threading.py", line 47, in test_context_manager
    raise error
KeyError: 'not_in_context_manager'

How REPRODUCIBLE (every time? intermittently? only in certain environments?): In general the the error would only happen in threaded environments where there is a requirement to override the default formatter using the context manager. In our case it was happening intermittently when testing a threaded part of our application which was logging the size of bitmath objects

What you EXPECTED to happen: For strings to be successfully formatted with the provided formatter to the context

What ACTUALLY happened: Some sizes where formatted with the formatter of another context.

VERSION of bitmath effected:

  • Version: 1.3.1.1
  • Install Source: PyPi

Your OPERATING SYSTEM and the affected PYTHON VERSION: unbuntu16.04 and python 2.7.14

walrusVision avatar Sep 12 '18 23:09 walrusVision

@walrusVision Thank you for filing a detailed bug report and for following the template. Deep down inside I think I knew that probably wasn't thread-safe, thanks for confirming it. The first thing I'll do is add a warning to the documentation.

tbielawa avatar Sep 13 '18 13:09 tbielawa

There we go, https://bitmath.readthedocs.io/en/latest/module.html#context-managers added a note there.

Going to leave this bug open for the time being. Maybe I or someone else will have an idea on how to make that work in threads.

tbielawa avatar Sep 13 '18 13:09 tbielawa

Thanks @tbielawa!

walrusVision avatar Sep 13 '18 21:09 walrusVision