From 0bcf3d84363361cd6da6d158b1794d45424f95a7 Mon Sep 17 00:00:00 2001
From: Santiago Gimeno <santiago.gimeno@ionide.es>
Date: Tue, 22 Dec 2015 14:56:03 +0100
Subject: [PATCH] src: multiple thread synchronization improvements

- Allow up to 4 the number of times SCardCancel is called before waiting for
  the status `thread` to finish.
- Destroy the mutexes and conds only in the destructors.
- Check that the thread handles are still valid before calling join on them.
---
 src/cardreader.cpp | 36 ++++++++++++++++++++----------------
 src/pcsclite.cpp   | 36 ++++++++++++++++++++++--------------
 2 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/src/cardreader.cpp b/src/cardreader.cpp
index 4f2ab89..30e92f5 100644
--- a/src/cardreader.cpp
+++ b/src/cardreader.cpp
@@ -68,14 +68,16 @@ CardReader::CardReader(const std::string &reader_name): m_card_context(0),
 }
 
 CardReader::~CardReader() {
-    SCardCancel(m_card_context);
-    int ret = uv_thread_join(&m_status_thread);
-    assert(ret == 0);
+    if (m_status_thread) {
+        SCardCancel(m_card_context);
+        assert(uv_thread_join(&m_status_thread) == 0);
+    }
 
     if (m_card_context) {
         SCardReleaseContext(m_card_context);
     }
 
+    uv_cond_destroy(&m_cond);
     uv_mutex_destroy(&m_mutex);
 }
 
@@ -302,21 +304,23 @@ NAN_METHOD(CardReader::Close) {
     LONG result = SCARD_S_SUCCESS;
     CardReader* obj = Nan::ObjectWrap::Unwrap<CardReader>(info.This());
 
-    uv_mutex_lock(&obj->m_mutex);
-    if (obj->m_state == 0) {
-        obj->m_state = 1;
-        do {
-            result = SCardCancel(obj->m_status_card_context);
-        } while (uv_cond_timedwait(&obj->m_cond, &obj->m_mutex, 10000000) != 0);
+    if (obj->m_status_thread) {
+        uv_mutex_lock(&obj->m_mutex);
+        if (obj->m_state == 0) {
+            int ret;
+            int times = 0;
+            obj->m_state = 1;
+            do {
+                result = SCardCancel(obj->m_status_card_context);
+                ret = uv_cond_timedwait(&obj->m_cond, &obj->m_mutex, 10000000);
+            } while ((ret != 0) && (++ times < 5));
+        }
+
+        assert(uv_thread_join(&obj->m_status_thread) == 0);
+        obj->m_status_thread = 0;
+        uv_mutex_unlock(&obj->m_mutex);
     }
 
-    assert(uv_thread_join(&obj->m_status_thread) == 0);
-    obj->m_status_thread = 0;
-
-    uv_mutex_unlock(&obj->m_mutex);
-    uv_mutex_destroy(&obj->m_mutex);
-    uv_cond_destroy(&obj->m_cond);
-
     info.GetReturnValue().Set(Nan::New<Number>(result));
 }
 
diff --git a/src/pcsclite.cpp b/src/pcsclite.cpp
index bc4af83..87e5191 100644
--- a/src/pcsclite.cpp
+++ b/src/pcsclite.cpp
@@ -53,13 +53,16 @@ PCSCLite::PCSCLite(): m_card_context(0),
 PCSCLite::~PCSCLite() {
 
     if (m_status_thread) {
-        int ret = uv_thread_join(&m_status_thread);
-        assert(ret == 0);
+        SCardCancel(m_card_context);
+        assert(uv_thread_join(&m_status_thread) == 0);
     }
 
     if (m_card_context) {
         SCardReleaseContext(m_card_context);
     }
+
+    uv_cond_destroy(&m_cond);
+    uv_mutex_destroy(&m_mutex);
 }
 
 NAN_METHOD(PCSCLite::New) {
@@ -96,23 +99,28 @@ NAN_METHOD(PCSCLite::Close) {
 
     LONG result = SCARD_S_SUCCESS;
     if (obj->m_pnp) {
-        uv_mutex_lock(&obj->m_mutex);
-        if (obj->m_state == 0) {
-            obj->m_state = 1;
-            do {
-                result = SCardCancel(obj->m_card_context);
-            } while (uv_cond_timedwait(&obj->m_cond, &obj->m_mutex, 10000000) != 0);
-        }
+        if (obj->m_status_thread) {
+            uv_mutex_lock(&obj->m_mutex);
+            if (obj->m_state == 0) {
+                int ret;
+                int times = 0;
+                obj->m_state = 1;
+                do {
+                    result = SCardCancel(obj->m_card_context);
+                    ret = uv_cond_timedwait(&obj->m_cond, &obj->m_mutex, 10000000);
+                } while ((ret != 0) && (++ times < 5));
+            }
 
-        uv_mutex_unlock(&obj->m_mutex);
-        uv_mutex_destroy(&obj->m_mutex);
-        uv_cond_destroy(&obj->m_cond);
+            uv_mutex_unlock(&obj->m_mutex);
+        }
     } else {
         obj->m_state = 1;
     }
 
-    assert(uv_thread_join(&obj->m_status_thread) == 0);
-    obj->m_status_thread = 0;
+    if (obj->m_status_thread) {
+        assert(uv_thread_join(&obj->m_status_thread) == 0);
+        obj->m_status_thread = 0;
+    }
 
     info.GetReturnValue().Set(Nan::New<Number>(result));
 }