Why is this singleton implementation "not thread safe"?

Question

1. The @Singleton decorator

I found an elegant way to decorate a Python class to make it a singleton. The class can only produce one object. Each Instance() call returns the same object:

class Singleton:
    """
    A non-thread-safe helper class to ease implementing singletons.
    This should be used as a decorator -- not a metaclass -- to the
    class that should be a singleton.

    The decorated class can define one `__init__` function that
    takes only the `self` argument. Also, the decorated class cannot be
    inherited from. Other than that, there are no restrictions that apply
    to the decorated class.

    To get the singleton instance, use the `Instance` method. Trying
    to use `__call__` will result in a `TypeError` being raised.

    """

    def __init__(self, decorated):
        self._decorated = decorated

    def Instance(self):
        """
        Returns the singleton instance. Upon its first call, it creates a
        new instance of the decorated class and calls its `__init__` method.
        On all subsequent calls, the already created instance is returned.

        """
        try:
            return self._instance
        except AttributeError:
            self._instance = self._decorated()
            return self._instance

    def __call__(self):
        raise TypeError('Singletons must be accessed through `Instance()`.')

    def __instancecheck__(self, inst):
        return isinstance(inst, self._decorated)

I found the code here: Is there a simple, elegant way to define singletons?

The comment at the top says:

[This is] a non-thread-safe helper class to ease implementing singletons.

Unfortunately, I don't have enough multithreading experience to see the 'thread-unsafeness' myself.

 

2. Questions

I'm using this @Singleton decorator in a multithreaded Python application. I'm worried about potential stability issues. Therefore:

1. Is there a way to make this code completely thread-safe?

2. If the previous question has no solution (or if its solution is too cumbersome), what precautions should I take to stay safe?

3. @Aran-Fey pointed out that the decorator is badly coded. Any improvements are of course very much appreciated.

---

_{Hereby I provide my current system settings:
    >  Python 3.6.3
    >  Windows 10, 64-bit}

Answer 1

You can also protect against potentially slow object initialization by separating your locks by class type (without modifying original object, which can be a problem in some cases when cPickle is used, etc), like this:

class Singleton(type):
    __instances = {}
    __lock = threading.Lock()

    @dataclass
    class _LockedObj:
        obj: any
        lock: threading.Lock

    def __call__(cls, *args, **kwargs):
        if cls not in cls.__instances:
            with cls.__lock:
                if cls not in cls.__instances:
                    cls.__instances[cls] = cls._LockedObj(None, threading.Lock())

        if not cls.__instances[cls].obj:
            with cls.__instances[cls].lock:
                if not cls.__instances[cls].obj:
                    cls.__instances[cls].obj = super(Singleton, cls).__call__(*args, **kwargs)
        return cls.__instances[cls].obj

Thus, the code below will run in 5 seconds, not 8

class SlowA(metaclass=Singleton):
    def __init__(self):
        print("Creating SlowA")
        time.sleep(5)
        print("Created SlowA")

class SlowB(metaclass=Singleton):
    def __init__(self):
        print("Creating SlowB")
        time.sleep(3)
        print("Created SlowB")


start = time.time()
threads = []
for klass in [SlowA, SlowB, SlowB, SlowA, SlowB]:
    t = threading.Thread(target=lambda: klass())
    t.start()
    threads.append(t)

[x.join() for x in threads]
print(time.time() - start)

Answer 2

I suggest you choose a better singleton implementation. The metaclass-based implementation is the most elegant in my opinion.

As for for thread-safety, neither your approach nor any of the ones suggested in the above link are thread safe: it is always possible that a thread reads that there is no existing instance and starts creating one, but another thread does the same before the first instance was stored.

You can use a with lock controller to protect the __call__ method of a metaclass-based singleton class with a lock.

import threading

lock = threading.Lock()

class Singleton(type):
    _instances = {}

    def __call__(cls, *args, **kwargs):
        if cls not in cls._instances:
            with lock:
                if cls not in cls._instances:
                    cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)
        return cls._instances[cls]


class SingletonClass(metaclass=Singleton):
    pass

As suggested by se7entyse7en, you can use a check-lock-check pattern. Since singletons are only created once, your only concern is that the creation of the initial instance must be locked. Although once this is done, retrieving the instance requires no lock at all. For that reason we accept the duplication of the check on the first call so that all further call do not even need to acquire the lock.

Answer 3

While providing thread-safety, the currently accepted answer has limitation as it can easily dead-lock.

For example, if both Class_1 and Class_2 implement that singleton pattern, calling the constructor of Class_1 in Class_2 (or vice versa) would dead-lock. This is due to the fact that all the classes implemented through that meta-class share the same lock.

After searching the internet for a better design, I found this one:

https://gist.github.com/wys1203/830f52c31151226599ac015b87b6e05c

It overcomes the dead-lock limitation by providing each class implemented through the meta-class with its own lock.

Answer 4

I'm posting this just to simplify suggested solution by @OlivierMelançon and @se7entyse7en: no overhead by import functools and wrapping.

import threading

lock = threading.Lock()

class SingletonOptmizedOptmized(type):
    _instances = {}
    def __call__(cls, *args, **kwargs):
        if cls not in cls._instances:
            with lock:
                if cls not in cls._instances:
                    cls._instances[cls] = super(SingletonOptmizedOptmized, cls).__call__(*args, **kwargs)
        return cls._instances[cls]

class SingletonClassOptmizedOptmized(metaclass=SingletonOptmizedOptmized):
    pass

Difference:

>>> timeit('SingletonClass()', globals=globals(), number=1000000)
0.4635776
>>> timeit('SingletonClassOptmizedOptmized()', globals=globals(), number=1000000)
0.192263300000036

Answer 5

If you're concerned about performance you could improve the solution of the accepted answer by using the check-lock-check pattern to minimize locking acquisition:

class SingletonOptmized(type):
    _instances = {}

    def __call__(cls, *args, **kwargs):
        if cls not in cls._instances:
            cls._locked_call(*args, **kwargs)
        return cls._instances[cls]

    @synchronized(lock)
    def _locked_call(cls, *args, **kwargs):
        if cls not in cls._instances:
            cls._instances[cls] = super(SingletonOptmized, cls).__call__(*args, **kwargs)

class SingletonClassOptmized(metaclass=SingletonOptmized):
    pass

Here's the difference:

In [9]: %timeit SingletonClass()
488 ns ± 4.67 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [10]: %timeit SingletonClassOptmized()
204 ns ± 4 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)