Thread-safe Singleton doesn't work. Python

Question

I tried to find the bug, described in this post, and i found it.

It's Singleton class, which i gave from StackOverflow from this post

When i start testing that class, i found, that programm fall down (just like described in first link)...

That's minimal example, that leads to bug:

Singleton.py

import threading


class Singleton(object):
    __singleton_lock = threading.Lock()
    __singleton_instance = None

    @classmethod
    def Instance(cls):
        if not cls.__singleton_instance:
            with cls.__singleton_lock:
                if not cls.__singleton_instance:
                    cls.__singleton_instance = cls()
        return cls.__singleton_instance

Data.py

#-*- coding: utf-8 -*-
from singletone import Singleton
import threading


class Data(Singleton):

    def __init__(self):
        self.task_table = {"Zadanie1.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie2.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie3.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie4.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie5.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie6.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie7.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie8.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie9.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie10.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie11.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie12.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie13.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie14.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie15.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie16.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie17.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie18.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie19.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie20.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie21.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie22.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie23.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie24.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie25.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie26.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie27.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie28.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie29.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie30.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie31.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie32.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie33.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie34.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie35.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie36.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie37.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie38.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie39.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie40.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie41.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie42.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie43.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie44.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie45.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie46.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie47.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie48.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie49.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie50.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie51.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie52.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie53.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie54.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie55.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie56.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie57.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie58.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie59.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie60.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie61.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie62.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie63.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie64.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie65.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie66.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie67.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie68.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie69.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie70.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie71.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie72.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie73.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie74.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie75.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie76.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie77.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie78.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie79.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie80.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie81.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie82.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie83.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie84.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie85.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie86.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie87.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie88.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie89.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie90.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie91.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie92.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie93.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie94.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie95.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie96.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie97.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie98.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie99.py": {'status': 'uncomplete', 'result': '', 'time': 0}, "Zadanie100.py": {'status': 'uncomplete', 'result': '', 'time': 0}}
        self.complete_task = threading.Event()
        self.complete_task.set()

    def getTotalCompleteTasks(self):
        result = 0
        for taskname in self.task_table.keys():
            if self.task_table[taskname]['status'] == 'complete':
                result += 1
        return result

Worker.py

#-*- coding: utf-8 -*-
import threading
import subprocess


class Worker(threading.Thread):

    def __init__(self, data, taskname):
        self.taskname = taskname
        self.data = data
        threading.Thread.__init__(self)

    def run(self):
        self.data.complete_task.clear()
        print u"Start executing %s" % self.taskname
        startupinfo = subprocess.STARTUPINFO()
        startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW
        startupinfo.wShowWindow = subprocess.SW_HIDE
        p = subprocess.Popen("C:\Python27\python.exe tasks100.5\\" + self.taskname, startupinfo=startupinfo, shell=False, stdout=subprocess.PIPE)
        job_result, err = p.communicate()
        print u"Stop executing %s" % self.taskname
        self.data.task_table[self.taskname]['status'] = 'complete'
        self.data.complete_task.set()

Agent.py

#-*- coding: utf-8 -*-
import communication_servers as cs
from worker import Worker
from data import Data
import time


app_data = Data.Instance()
SRV = cs.Server(app_data)
SRV.setDaemon(True)
SRV.start()

my_tasks = app_data.task_table.keys()[:]


while True:
    print u"Complete tasks: %d\n" % app_data.getTotalCompleteTasks()
    time.sleep(1)

    if my_tasks:
        if app_data.complete_task.is_set():
            job = Worker(app_data, my_tasks.pop())
            job.setDaemon(True)
            print u"Running"
            job.start()
        else:
            print u"Can't run"
        print u"Test string"

Communication_server.py

#-*- coding: utf-8 -*-
import threading
import time


class Server(threading.Thread):

    def __init__(self, data):
        threading.Thread.__init__(self)

    def run(self):
        while True:
            print('Server still working...\n')
            time.sleep(5)

All files like ZadanieXX.py is just there:

import time
i = 2
p = i**400
time.sleep(5)
print p

There is only reduce version of real code. And it's leads to bug just about in 1 case of 30 starts. The bug is when main while loop im Agent.py doesn't iterate or iterates, but complete_task event never sets (in less then 1 case of 100 starts).

p.s. I tried to use this Singleton pattern, but bug is still here...

Any ideas, why it working in that way? Any proposal, how i can create single-instanced, thread-safety class to store all application working data?

My english is not very good. Sorry.

Answer 1

Note that your use of locking is actually not thread safe. There is a race condition between checking whether an instance exists and locking access to the instance:

def Instance(cls):
    if not cls.__singleton_instance:
        # another thread may succeed with the previous check again here
        with cls.__singleton_lock:
            if not cls.__singleton_instance:
                cls.__singleton_instance = cls()
    return cls.__singleton_instance

You should swap acquiring of the lock and the check of __singleton_instance, since the first guards the later.

Note that in your minimal working example, you are constructing the Singleton explicitly, so the race condition is never trigger (there is no multi-threaded instantiation). In fact, you example allows you to work without a singleton, since you are explicitly instantiating your class only once.

Answer 2

Sorry about this is not an answer about Singleton DP, but I think there is no reason that you have to use Singleton.

In your sample, Data class is just used as data storage, and you can make single data storage, using python's own module Queue.

8.10. Queue - A synchronized queue class

The Queue module implements multi-producer, multi-consumer queues. It is especially useful in threaded programming when information must be exchanged safely between multiple threads. The Queue class in this module implements all the required locking semantics. It depends on the availability of thread support in Python; see the threading module.

Yes. It's thread safe so you can Queue as your single, centered data storage.

Check something like this.

#-*- coding: utf-8 -*-
from Queue import Queue
from threading import Thread
import time

# Set up some global variables
num_threads = 2
data_queue = Queue()

# Your Data
files = ['Zadanie1.py', 'Zadanie2.py', ... ]

def doSomething(i, q):
    while True:
        print '[{}] Waiting...'.format(i)
        filename = q.get()
        print '[{}] Working: {} '.format(i, filename)
        # Do something you need
        time.sleep(i + random.randint(1, 5))
        q.task_done()

# Set up threads
for i in range(num_threads):
    worker = Thread(target=doSomething, args=(i, data_queue,))
    worker.setDaemon(True)
    worker.start()
    # Note that you can start threads before filling queue,
    # because queue.get() will block until queue has data to return.

# Fill queue
for filename in files:
    print 'Queuing: {}'.format(filename)
    data_queue.put(filename)

# Wait for the queue to be empty.
data_queue.join()
print 'Done!'

Result will be like this.

[0] Waiting...
[1] Waiting...
Queuing: Zadanie1.py
Queuing: Zadanie2.py
...
[1] Working: Zadanie1.py 
[0] Working: Zadanie2.py 
...
[0] Waiting...
[1] Waiting...
Done!

Hope this helpful!.