You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2023/01/20 14:47:34 UTC

[nuttx-apps] 02/02: system/media: fix race condion on pthread id

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx-apps.git

commit 0bf87e7893ea409edc24f7f78532e1632080262b
Author: ligd <li...@xiaomi.com>
AuthorDate: Tue Jan 3 20:35:46 2023 +0800

    system/media: fix race condion on pthread id
    
    Signed-off-by: ligd <li...@xiaomi.com>
---
 system/nxcamera/nxcamera.c     | 56 +++++++++++++++++++++++++++---------------
 system/nxlooper/nxlooper.c     | 52 +++++++++++++++++++++++++--------------
 system/nxplayer/nxplayer.c     | 52 +++++++++++++++++++++++++--------------
 system/nxrecorder/nxrecorder.c | 52 +++++++++++++++++++++++++--------------
 4 files changed, 138 insertions(+), 74 deletions(-)

diff --git a/system/nxcamera/nxcamera.c b/system/nxcamera/nxcamera.c
index c1d90ac48..5cb9c1341 100644
--- a/system/nxcamera/nxcamera.c
+++ b/system/nxcamera/nxcamera.c
@@ -167,6 +167,36 @@ static int nxcamera_opendevice(FAR struct nxcamera_s *pcam)
   return -ENODEV;
 }
 
+/****************************************************************************
+ * Name: nxcameraer_jointhread
+ ****************************************************************************/
+
+static void nxcamera_jointhread(FAR struct nxcamera_s *pcam)
+{
+  FAR void *value;
+  int id = 0;
+
+  if (gettid() == pcam->loop_id)
+    {
+      return;
+    }
+
+  pthread_mutex_lock(&pcam->mutex);
+
+  if (pcam->loop_id > 0)
+    {
+      id = pcam->loop_id;
+      pcam->loop_id = 0;
+    }
+
+  pthread_mutex_unlock(&pcam->mutex);
+
+  if (id > 0)
+    {
+      pthread_join(id, &value);
+    }
+}
+
 /****************************************************************************
  * Name: nxcamera_loopthread
  *
@@ -477,7 +507,6 @@ int nxcamera_setfile(FAR struct nxcamera_s *pcam, FAR const char *pfile,
 int nxcamera_stop(FAR struct nxcamera_s *pcam)
 {
   struct video_msg_s term_msg;
-  FAR void           *value;
 
   DEBUGASSERT(pcam != NULL);
 
@@ -501,8 +530,7 @@ int nxcamera_stop(FAR struct nxcamera_s *pcam)
 
   /* Join the thread.  The thread will do all the cleanup. */
 
-  pthread_join(pcam->loop_id, &value);
-  pcam->loop_id = 0;
+  nxcamera_jointhread(pcam);
 
   return OK;
 }
@@ -536,7 +564,6 @@ int nxcamera_stream(FAR struct nxcamera_s *pcam,
   struct mq_attr             attr;
   struct sched_param         sparam;
   pthread_attr_t             tattr;
-  FAR void                   *value;
   int                        ret;
   int                        i;
   struct v4l2_buffer         buf;
@@ -681,10 +708,7 @@ int nxcamera_stream(FAR struct nxcamera_s *pcam,
    * to perform clean-up.
    */
 
-  if (pcam->loop_id != 0)
-    {
-      pthread_join(pcam->loop_id, &value);
-    }
+  nxcamera_jointhread(pcam);
 
   pthread_attr_init(&tattr);
   sparam.sched_priority = sched_get_priority_max(SCHED_FIFO) - 9;
@@ -792,23 +816,15 @@ FAR struct nxcamera_s *nxcamera_create(void)
 
 void nxcamera_release(FAR struct nxcamera_s *pcam)
 {
-  FAR void *value;
   int      refcount;
 
-  /* Lock the mutex */
-
-  pthread_mutex_lock(&pcam->mutex);
-
   /* Check if there was a previous thread and join it if there was */
 
-  if (pcam->loop_id != 0)
-    {
-      pthread_mutex_unlock(&pcam->mutex);
-      pthread_join(pcam->loop_id, &value);
-      pcam->loop_id = 0;
+  nxcamera_jointhread(pcam);
 
-      pthread_mutex_lock(&pcam->mutex);
-    }
+  /* Lock the mutex */
+
+  pthread_mutex_lock(&pcam->mutex);
 
   /* Reduce the reference count */
 
diff --git a/system/nxlooper/nxlooper.c b/system/nxlooper/nxlooper.c
index 90cec746e..3e03afea4 100644
--- a/system/nxlooper/nxlooper.c
+++ b/system/nxlooper/nxlooper.c
@@ -308,6 +308,36 @@ static int nxlooper_enqueueplaybuffer(FAR struct nxlooper_s *plooper,
   return OK;
 }
 
+/****************************************************************************
+ * Name: nxlooper_jointhread
+ ****************************************************************************/
+
+static void nxlooper_jointhread(FAR struct nxlooper_s *plooper)
+{
+  FAR void *value;
+  int id = 0;
+
+  if (gettid() == plooper->loop_id)
+    {
+      return;
+    }
+
+  pthread_mutex_lock(&plooper->mutex);
+
+  if (plooper->loop_id > 0)
+    {
+      id = plooper->loop_id;
+      plooper->loop_id = 0;
+    }
+
+  pthread_mutex_unlock(&plooper->mutex);
+
+  if (id > 0)
+    {
+      pthread_join(id, &value);
+    }
+}
+
 /****************************************************************************
  * Name: nxlooper_thread_loopthread
  *
@@ -904,7 +934,6 @@ int nxlooper_setdevice(FAR struct nxlooper_s *plooper,
 int nxlooper_stop(FAR struct nxlooper_s *plooper)
 {
   struct audio_msg_s term_msg;
-  FAR void           *value;
 
   DEBUGASSERT(plooper != NULL);
 
@@ -928,8 +957,7 @@ int nxlooper_stop(FAR struct nxlooper_s *plooper)
 
   /* Join the thread.  The thread will do all the cleanup. */
 
-  pthread_join(plooper->loop_id, &value);
-  plooper->loop_id = 0;
+  nxlooper_jointhread(plooper);
 
   return OK;
 }
@@ -966,7 +994,6 @@ int nxlooper_loopraw(FAR struct nxlooper_s *plooper,
   pthread_attr_t           tattr;
   struct audio_caps_desc_s cap_desc;
   struct ap_buffer_info_s  buf_info;
-  FAR void                 *value;
   int                      ret;
 
   DEBUGASSERT(plooper != NULL);
@@ -1098,10 +1125,7 @@ int nxlooper_loopraw(FAR struct nxlooper_s *plooper,
    * to perform clean-up.
    */
 
-  if (plooper->loop_id != 0)
-    {
-      pthread_join(plooper->loop_id, &value);
-    }
+  nxlooper_jointhread(plooper);
 
   pthread_attr_init(&tattr);
   sparam.sched_priority = sched_get_priority_max(SCHED_FIFO) - 9;
@@ -1223,20 +1247,12 @@ FAR struct nxlooper_s *nxlooper_create(void)
 void nxlooper_release(FAR struct nxlooper_s *plooper)
 {
   int      refcount;
-  FAR void *value;
-
-  pthread_mutex_lock(&plooper->mutex);
 
   /* Check if there was a previous thread and join it if there was */
 
-  if (plooper->loop_id != 0)
-    {
-      pthread_mutex_unlock(&plooper->mutex);
-      pthread_join(plooper->loop_id, &value);
-      plooper->loop_id = 0;
+  nxlooper_jointhread(plooper);
 
-      pthread_mutex_lock(&plooper->mutex);
-    }
+  pthread_mutex_lock(&plooper->mutex);
 
   /* Reduce the reference count */
 
diff --git a/system/nxplayer/nxplayer.c b/system/nxplayer/nxplayer.c
index bde36980d..c8f534e4e 100644
--- a/system/nxplayer/nxplayer.c
+++ b/system/nxplayer/nxplayer.c
@@ -718,6 +718,36 @@ static int nxplayer_enqueuebuffer(FAR struct nxplayer_s *pplayer,
   return OK;
 }
 
+/****************************************************************************
+ * Name: nxplayer_jointhread
+ ****************************************************************************/
+
+static void nxplayer_jointhread(FAR struct nxplayer_s *pplayer)
+{
+  FAR void *value;
+  int id = 0;
+
+  if (gettid() == pplayer->play_id)
+    {
+      return;
+    }
+
+  pthread_mutex_lock(&pplayer->mutex);
+
+  if (pplayer->play_id > 0)
+    {
+      id = pplayer->play_id;
+      pplayer->play_id = 0;
+    }
+
+  pthread_mutex_unlock(&pplayer->mutex);
+
+  if (id > 0)
+    {
+      pthread_join(id, &value);
+    }
+}
+
 /****************************************************************************
  * Name: nxplayer_thread_playthread
  *
@@ -1670,7 +1700,6 @@ int nxplayer_setdevice(FAR struct nxplayer_s *pplayer,
 int nxplayer_stop(FAR struct nxplayer_s *pplayer)
 {
   struct audio_msg_s term_msg;
-  FAR void           *value;
 
   DEBUGASSERT(pplayer != NULL);
 
@@ -1694,8 +1723,7 @@ int nxplayer_stop(FAR struct nxplayer_s *pplayer)
 
   /* Join the thread.  The thread will do all the cleanup. */
 
-  pthread_join(pplayer->play_id, &value);
-  pplayer->play_id = 0;
+  nxplayer_jointhread(pplayer);
 
   return OK;
 }
@@ -1738,7 +1766,6 @@ static int nxplayer_playinternal(FAR struct nxplayer_s *pplayer,
   struct mq_attr      attr;
   struct sched_param  sparam;
   pthread_attr_t      tattr;
-  FAR void           *value;
   struct audio_caps_desc_s cap_desc;
   struct ap_buffer_info_s  buf_info;
 #ifdef CONFIG_NXPLAYER_INCLUDE_MEDIADIR
@@ -1936,10 +1963,7 @@ static int nxplayer_playinternal(FAR struct nxplayer_s *pplayer,
    * to perform clean-up.
    */
 
-  if (pplayer->play_id != 0)
-    {
-      pthread_join(pplayer->play_id, &value);
-    }
+  nxplayer_jointhread(pplayer);
 
   /* Start the playfile thread to stream the media file to the
    * audio device.
@@ -2159,20 +2183,12 @@ FAR struct nxplayer_s *nxplayer_create(void)
 void nxplayer_release(FAR struct nxplayer_s *pplayer)
 {
   int         refcount;
-  FAR void    *value;
-
-  pthread_mutex_lock(&pplayer->mutex);
 
   /* Check if there was a previous thread and join it if there was */
 
-  if (pplayer->play_id != 0)
-    {
-      pthread_mutex_unlock(&pplayer->mutex);
-      pthread_join(pplayer->play_id, &value);
-      pplayer->play_id = 0;
+  nxplayer_jointhread(pplayer);
 
-      pthread_mutex_lock(&pplayer->mutex);
-    }
+  pthread_mutex_lock(&pplayer->mutex);
 
   /* Reduce the reference count */
 
diff --git a/system/nxrecorder/nxrecorder.c b/system/nxrecorder/nxrecorder.c
index 83fdd3186..9b6cb4372 100644
--- a/system/nxrecorder/nxrecorder.c
+++ b/system/nxrecorder/nxrecorder.c
@@ -215,6 +215,36 @@ static int nxrecorder_enqueuebuffer(FAR struct nxrecorder_s *precorder,
   return OK;
 }
 
+/****************************************************************************
+ * Name: nxrecorder_jointhread
+ ****************************************************************************/
+
+static void nxrecorder_jointhread(FAR struct nxrecorder_s *precorder)
+{
+  FAR void *value;
+  int id = 0;
+
+  if (gettid() == precorder->record_id)
+    {
+      return;
+    }
+
+  pthread_mutex_lock(&precorder->mutex);
+
+  if (precorder->record_id > 0)
+    {
+      id = precorder->record_id;
+      precorder->record_id = 0;
+    }
+
+  pthread_mutex_unlock(&precorder->mutex);
+
+  if (id > 0)
+    {
+      pthread_join(id, &value);
+    }
+}
+
 /****************************************************************************
  * Name: nxrecorder_thread_recordthread
  *
@@ -710,7 +740,6 @@ int nxrecorder_setdevice(FAR struct nxrecorder_s *precorder,
 int nxrecorder_stop(FAR struct nxrecorder_s *precorder)
 {
   struct audio_msg_s term_msg;
-  FAR void           *value;
 
   DEBUGASSERT(precorder != NULL);
 
@@ -734,8 +763,7 @@ int nxrecorder_stop(FAR struct nxrecorder_s *precorder)
 
   /* Join the thread.  The thread will do all the cleanup. */
 
-  pthread_join(precorder->record_id, &value);
-  precorder->record_id = 0;
+  nxrecorder_jointhread(precorder);
 
   return OK;
 }
@@ -773,7 +801,6 @@ int nxrecorder_recordraw(FAR struct nxrecorder_s *precorder,
   pthread_attr_t           tattr;
   struct audio_caps_desc_s cap_desc;
   struct ap_buffer_info_s  buf_info;
-  FAR void                 *value;
   int                      ret;
 
   DEBUGASSERT(precorder != NULL);
@@ -884,10 +911,7 @@ int nxrecorder_recordraw(FAR struct nxrecorder_s *precorder,
    * to perform clean-up.
    */
 
-  if (precorder->record_id != 0)
-    {
-      pthread_join(precorder->record_id, &value);
-    }
+  nxrecorder_jointhread(precorder);
 
   /* Start the recordfile thread to stream the media file to the
    * audio device.
@@ -995,20 +1019,12 @@ FAR struct nxrecorder_s *nxrecorder_create(void)
 void nxrecorder_release(FAR struct nxrecorder_s *precorder)
 {
   int         refcount;
-  FAR void    *value;
-
-  pthread_mutex_lock(&precorder->mutex);
 
   /* Check if there was a previous thread and join it if there was */
 
-  if (precorder->record_id != 0)
-    {
-      pthread_mutex_unlock(&precorder->mutex);
-      pthread_join(precorder->record_id, &value);
-      precorder->record_id = 0;
+  nxrecorder_jointhread(precorder);
 
-      pthread_mutex_lock(&precorder->mutex);
-    }
+  pthread_mutex_lock(&precorder->mutex);
 
   /* Reduce the reference count */