Merge pull request #1675 from GeoBaltz/fix26

DO NOT MERGE!

include/hamlib/rig.h

@@ -2891,6 +2891,7 @@ struct rig_state {
     struct timespec freq_event_elapsed;
     int freq_skip; /*!< allow frequency skip for gpredict RX/TX freq set */
     client_t client;
+    pthread_mutex_t api_mutex;   // Lock for any API entry
 // New rig_state items go before this line ============================================
 };
 

src/rig.c

@@ -171,11 +171,21 @@ const char hamlib_copyright[231] = /* hamlib 1.2 ABI specifies 231 bytes */
 
 #define ICOM_EXCEPTIONS (rig->caps->rig_model == RIG_MODEL_IC9700 || rig->caps->rig_model == RIG_MODEL_IC9100 || rig->caps->rig_model == RIG_MODEL_IC910)
 
+// If the OS/library supports it, use a recursive mutex for the main lock.
+// This eliminates depth races, and guards against multiple app threads, too.
+// Set define to 0 to use depth-based locking. It should be deduced from the
+//   environment, but I can't find a fine-grained enough parameter. Should be
+//   OK on any POSIX-2017 or later system.
+#define USE_RECURSIVE_MUTEX 1
+#if USE_RECURSIVE_MUTEX
+#define LOCK(n) rig_lock(rig,n)
+#else
 // The LOCK macro is for the primary thread calling the rig functions
 // For a separate thread use rig_lock directly
 // The purpose here is to avoid deadlock during recursion
 // Any other thread should grab the mutex itself via rig_lock
 #define LOCK(n) if (STATE(rig)->depth == 1) { rig_debug(RIG_DEBUG_CACHE, "%s: %s\n", n?"lock":"unlock", __func__);  rig_lock(rig,n); }
+#endif
 
 MUTEX(morse_mutex);
 
@@ -621,7 +631,7 @@ RIG *HAMLIB_API rig_init(rig_model_t rig_model)
     rs->rig_model = caps->rig_model;
     rs->priv = NULL;
     rs->async_data_enabled = 0;
-    rs->depth = 1;
+    //    rs->depth = 1;
     rs->comm_state = 0;
     rs->comm_status = RIG_COMM_STATUS_CONNECTING;
     rs->tuner_control_pathname = DEFAULT_TUNER_CONTROL_PATHNAME;
@@ -891,8 +901,20 @@ RIG *HAMLIB_API rig_init(rig_model_t rig_model)
     // we have to copy rs to rig->state_deprecated for DLL backwards compatibility
     memcpy(&rig->state_deprecated, rs, sizeof(rig->state_deprecated));
 
+    // Set up lock for any API entry point
+    // If available, use a recursive mutex. Else, fall back on the
+    //   depth count.
+    pthread_mutexattr_t api_attr;
+    pthread_mutexattr_init(&api_attr);
+#if USE_RECURSIVE_MUTEX
+    pthread_mutexattr_settype(&api_attr, PTHREAD_MUTEX_RECURSIVE);
+    HAMLIB_TRACE;
+#endif
+    pthread_mutex_init(&rs->api_mutex, &api_attr);
+    pthread_mutexattr_destroy(&api_attr);
+
     /*
-     * let the backend a chance to setup his private data
+     * Give the backend a chance to setup his private data
      * This must be done only once defaults are setup,
      * so the backend init can override rig_state.
      */
@@ -1858,7 +1880,10 @@ int HAMLIB_API rig_cleanup(RIG *rig)
         rig->caps->rig_cleanup(rig);
     }
 
-    //TODO Release and null any allocated port structures
+    //pthread_mutex_destroy(&STATE(rig)->api_mutex);
+
+    //TODO Release and null any allocated data -
+    // state, ports, cache, etc.
 
     free(rig);
 
@@ -2494,14 +2519,6 @@ int HAMLIB_API rig_get_freq(RIG *rig, vfo_t vfo, freq_t *freq)
 
     rig_debug(RIG_DEBUG_CACHE, "%s: depth=%d\n", __func__, rs->depth);
 
-    if (rs->depth == 1)
-    {
-        rig_debug(RIG_DEBUG_CACHE, "%s: %s\n", 1 ? "lock" : "unlock", __func__);
-//        rig_lock(rig, 1);
-    }
-
-
-
     // there are some rigs that can't get VFOA freq while VFOB is transmitting
     // so we'll return the cached VFOA freq for them
     // should we use the cached ptt maybe? No -- we have to be 100% sure we're in PTT to ignore this request
@@ -2789,6 +2806,7 @@ int HAMLIB_API rig_set_mode(RIG *rig, vfo_t vfo, rmode_t mode, pbwidth_t width)
     if (locked_mode)
     {
         ELAPSED2;
+        LOCK(0);
         RETURNFUNC(RIG_OK);
     }
 
@@ -2797,6 +2815,7 @@ int HAMLIB_API rig_set_mode(RIG *rig, vfo_t vfo, rmode_t mode, pbwidth_t width)
     {
         rig_debug(RIG_DEBUG_VERBOSE, "%s PTT on so set_mode ignored\n", __func__);
         ELAPSED2;
+        LOCK(0);
         RETURNFUNC(RIG_OK);
     }
 
@@ -2805,6 +2824,7 @@ int HAMLIB_API rig_set_mode(RIG *rig, vfo_t vfo, rmode_t mode, pbwidth_t width)
     if (caps->set_mode == NULL)
     {
         ELAPSED2;
+        LOCK(0);
         RETURNFUNC(-RIG_ENAVAIL);
     }
 
@@ -3504,6 +3524,7 @@ int HAMLIB_API rig_get_vfo(RIG *rig, vfo_t *vfo)
             {
                 rig->caps->get_vfo = NULL;
                 *vfo = RIG_VFO_A;
+                LOCK(0);
                 RETURNFUNC(RIG_OK);
             }
 
@@ -3724,6 +3745,7 @@ int HAMLIB_API rig_set_ptt(RIG *rig, vfo_t vfo, ptt_t ptt)
                           __func__,
                           pttp->pathname);
                 ELAPSED2;
+                LOCK(0);
                 RETURNFUNC(-RIG_EIO);
             }
 
@@ -3735,6 +3757,7 @@ int HAMLIB_API rig_set_ptt(RIG *rig, vfo_t vfo, ptt_t ptt)
             if (RIG_OK != retcode)
             {
                 ELAPSED2;
+                LOCK(0);
                 RETURNFUNC(retcode);
             }
         }
@@ -3774,6 +3797,7 @@ int HAMLIB_API rig_set_ptt(RIG *rig, vfo_t vfo, ptt_t ptt)
                           __func__,
                           pttp->pathname);
                 ELAPSED2;
+                LOCK(0);
                 RETURNFUNC(-RIG_EIO);
             }
 
@@ -3786,6 +3810,7 @@ int HAMLIB_API rig_set_ptt(RIG *rig, vfo_t vfo, ptt_t ptt)
             {
                 rig_debug(RIG_DEBUG_ERR, "%s: ser_set_dtr retcode=%d\n", __func__, retcode);
                 ELAPSED2;
+                LOCK(0);
                 RETURNFUNC(retcode);
             }
         }
@@ -3825,6 +3850,7 @@ int HAMLIB_API rig_set_ptt(RIG *rig, vfo_t vfo, ptt_t ptt)
         rig_debug(RIG_DEBUG_WARN, "%s: unknown PTT type=%d\n", __func__,
                   pttp->type.ptt);
         ELAPSED2;
+        LOCK(0);
         RETURNFUNC(-RIG_EINVAL);
     }
 
@@ -3849,7 +3875,7 @@ int HAMLIB_API rig_set_ptt(RIG *rig, vfo_t vfo, ptt_t ptt)
     if (rs->post_ptt_delay > 0) { hl_usleep(rs->post_ptt_delay * 1000); }
 
     ELAPSED2;
-
+    LOCK(0);
     RETURNFUNC(retcode);
 }
 
@@ -8364,23 +8390,29 @@ void rig_lock(RIG *rig, int lock)
 
     struct rig_state *rs = STATE(rig);
 
-    if (rs->multicast == NULL) { return; } // not initialized yet
+#if 0
+    if (rs->multicast == NULL)
+    {
+        rig_debug(RIG_DEBUG_BUG, "%s: locking skipped, lock = %d\n", __func__, lock); 
+        return;
+    } // not initialized yet
 
     if (!rs->multicast->mutex_initialized)
     {
         rs->multicast->mutex = initializer;
         rs->multicast->mutex_initialized = 1;
     }
+#endif
 
     if (lock)
     {
-        pthread_mutex_lock(&rs->multicast->mutex);
+        pthread_mutex_lock(&rs->api_mutex);
         rig_debug(RIG_DEBUG_VERBOSE, "%s: client lock engaged\n", __func__);
     }
     else
     {
         rig_debug(RIG_DEBUG_VERBOSE, "%s: client lock disengaged\n", __func__);
-        pthread_mutex_unlock(&rs->multicast->mutex);
+        pthread_mutex_unlock(&rs->api_mutex);
     }
 
 #endif