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

I suggest you choose a better singleton implementation. The metaclass-based implementation is the most frequently used.

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 2

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)

Answer 3

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