LudovicRousseau / pyscard

pyscard smartcard library for python
http://pyscard.sourceforge.net/
GNU Lesser General Public License v2.1
377 stars 108 forks source link

pyscard seems to be not thread save #162

Closed Skill3t closed 4 months ago

Skill3t commented 4 months ago

The CardMonitor is implemented as singleton, so I want to add in different thread new observers. But if I do this really quickly the CardMonitor contains only one CardObserver

import threading as th
import signal
import time
from smartcard.CardMonitoring import CardMonitor, CardObserver

# this is a min example to shows that pyscard is not threads save
# If this skript is started you will see two  threads and a single
# CardMonitor (singleton) the CardMonitor has two observers (one from each thread)
# Represented by len(self.card_monitor.instance.obs)
# If use_staggered_observer_boot is set to False. Both Threads start the CardMonitor
# nearly at the same time and then the CardMonitor only have one observer'''

class PCSCDTaskThread(th.Thread):
    def __init__(self, exit_event, start_pcsc_event, number, use_staggered_observer_boot, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.exit_event = exit_event
        self.start_pcsc_event = start_pcsc_event
        self.card_monitor = None
        self.number = number
        self.my_name = ""
        self.use_staggered_observer_boot = use_staggered_observer_boot

    def run(self):
        self.my_name = th.current_thread().name
        print(f"Thread: {self.my_name} running")
        while True:
            if self.start_pcsc_event.is_set():
                if self.use_staggered_observer_boot:
                    time.sleep(self.number)
                if self.card_monitor is None:
                    self.card_monitor = CardMonitor()
                    card_observer = CardObserver()
                    self.card_monitor.addObserver(card_observer)
                if self.exit_event.is_set():
                    return
                print(f"len of card_monitor obs {len(self.card_monitor.instance.obs)}")
                print(f"Thread: {self.my_name} going to sleep for 5 s")
                time.sleep(5)

def main(use_staggered_observer_boot):
    def handler(signum, frame):
        exit_event.set()

    exit_event = th.Event()
    start_pcsc_event = th.Event()

    threads = []
    for i in range(2):
        pcsc_thread1 = PCSCDTaskThread(
                exit_event,
            start_pcsc_event,
            i,
            use_staggered_observer_boot=use_staggered_observer_boot
        )
        pcsc_thread1.start()
        threads.append(pcsc_thread1)
    time.sleep(3)
    start_pcsc_event.set()
    signal.signal(signal.SIGINT, handler)
    signal.signal(signal.SIGTERM, handler)

if __name__ == "__main__":
    main(use_staggered_observer_boot=True)

If you start the script you will see 2 observers. But when you change use_staggered_observer_boot=False only one observers is in the CardMonitor

So my questtion is: is pyscard thread save? If not where can i find this information in the documatation or could this be addes?

thanks a lot in advance

LudovicRousseau commented 4 months ago

This patch should fix the problem

--- a/smartcard/CardMonitoring.py
+++ b/smartcard/CardMonitoring.py
@@ -28,7 +28,7 @@ along with pyscard; if not, write to the Free Software
 Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 """

-from threading import Thread, Event
+from threading import Thread, Event, Lock
 from time import sleep
 import traceback

@@ -123,10 +123,12 @@ class CardMonitor(object):

     # the singleton
     instance = None
+    lock = Lock()

     def __init__(self):
-        if not CardMonitor.instance:
-            CardMonitor.instance = CardMonitor.__CardMonitorSingleton()
+        with CardMonitor.lock:
+            if not CardMonitor.instance:
+                CardMonitor.instance = CardMonitor.__CardMonitorSingleton()

     def __getattr__(self, name):
         return getattr(self.instance, name)

Please test and confirm it is OK for you.

Skill3t commented 4 months ago

Hey Ludovic, thanks a lot yes this fix the Problem. To be honest for my Produktion setup I will use pyscard single thread to avoid this type of problem.