diff --git a/include/hamlib/rig.h b/include/hamlib/rig.h index 6db75e359..ea6bd8db0 100644 --- a/include/hamlib/rig.h +++ b/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 ============================================ }; diff --git a/src/rig.c b/src/rig.c index 6e772fcf0..5234e3f3c 100644 --- a/src/rig.c +++ b/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