RDKB-64487: Code changes for thread hardening and safety under concur…

source/bulkdata/profile.c

@@ -1286,7 +1286,11 @@ T2ERROR deleteAllProfiles(bool delFromDisk)
     int profileIndex = 0;
     Profile *tempProfile = NULL;
 
-    pthread_rwlock_rdlock(&profileListLock);
+    /* Acquire wrlock once to atomically validate, read count, and disable all
+     * profiles.  This eliminates the TOCTOU race between the old pattern of
+     * rdlock-for-count then wrlock-per-profile, where the list could change
+     * between the two operations. */
+    pthread_rwlock_wrlock(&profileListLock);
     if(profileList == NULL)
     {
         T2Error("profile list is not initialized yet or profileList is empty, ignoring\n");
@@ -1295,14 +1299,23 @@ T2ERROR deleteAllProfiles(bool delFromDisk)
     }
 
     count = Vector_Size(profileList);
-    pthread_rwlock_unlock(&profileListLock);
-
-    for(; profileIndex < count; profileIndex++)
+    for(profileIndex = 0; profileIndex < count; profileIndex++)
     {
-        pthread_rwlock_wrlock(&profileListLock);
         tempProfile = (Profile *)Vector_At(profileList, profileIndex);
         tempProfile->enable = false;
         tempProfile->isSchedulerstarted = false;
+    }
+    pthread_rwlock_unlock(&profileListLock);
+
+    /* Second pass: perform blocking operations (unregister, signal, join) without
+     * holding profileListLock for extended periods.  The caller (uninitProfileList)
+     * sets initialized=false before calling, which prevents concurrent list
+     * modifications.  We still briefly acquire rdlock for each Vector_At to
+     * satisfy Coverity's lock-consistency analysis. */
+    for(profileIndex = 0; profileIndex < count; profileIndex++)
+    {
+        pthread_rwlock_rdlock(&profileListLock);
+        tempProfile = (Profile *)Vector_At(profileList, profileIndex);
         pthread_rwlock_unlock(&profileListLock);
 
         if(T2ERROR_SUCCESS != unregisterProfileFromScheduler(tempProfile->name))
@@ -1330,13 +1343,13 @@ T2ERROR deleteAllProfiles(bool delFromDisk)
              * after setting threadExists = false (see CollectAndReport cleanup). */
         }
 
-        /* Re-acquire profileListLock for profile cleanup */
-        pthread_rwlock_wrlock(&profileListLock);
+        /* grepSeekProfile cleanup is safe without profileListLock here:
+         * the profile's thread has been joined (or never existed), and
+         * initialized=false prevents concurrent access from other threads. */
         if(tempProfile->grepSeekProfile)
         {
             freeGrepSeekProfile(tempProfile->grepSeekProfile);
         }
-        pthread_rwlock_unlock(&profileListLock);
         if(delFromDisk == true)
         {
             removeProfileFromDisk(REPORTPROFILES_PERSISTENCE_PATH, tempProfile->name);

source/bulkdata/profilexconf.c

@@ -687,28 +687,31 @@ T2ERROR ProfileXConf_uninit()
     }
     initialized = false;
 
-    pthread_mutex_lock(&singleProfile->reportInProgressMutex);
-    bool reportInProgress = singleProfile->reportInProgress;
-    pthread_mutex_unlock(&singleProfile->reportInProgressMutex);
-    if(reportInProgress)
+    /* Wake the report thread (if it exists) and join it before cleanup.
+     * The thread may be in one of two states:
+     *   (a) reportInProgress=true  — actively generating a report
+     *   (b) reportInProgress=false — idle in pthread_cond_wait(&reuseThread)
+     * In both cases we must signal and join; otherwise case (b) would skip
+     * the join, and freeProfileXConf + pthread_cond_destroy would destroy
+     * resources the sleeping thread still references (undefined behavior). */
+    pthread_mutex_lock(&xconfProfileLock);
+    if(reportThreadExits)
     {
-        T2Debug("Waiting for final report before uninit\n");
-        pthread_mutex_lock(&xconfProfileLock);
         pthread_cond_signal(&reuseThread);
         pthread_mutex_unlock(&xconfProfileLock);
         pthread_join(singleProfile->reportThread, NULL);
-        pthread_mutex_lock(&singleProfile->reportInProgressMutex);
-        singleProfile->reportInProgress = false ;
-        pthread_mutex_unlock(&singleProfile->reportInProgressMutex);
-        T2Info("Final report is completed, releasing profile memory\n");
+        pthread_mutex_lock(&xconfProfileLock);
+        singleProfile->reportInProgress = false;
+        T2Info("Report thread joined, releasing profile memory\n");
     }
-    pthread_mutex_lock(&xconfProfileLock);
     freeProfileXConf();
     pthread_mutex_unlock(&xconfProfileLock);
 
-    /* Destroy condition variable at module uninit, after all threads are stopped */
+    /* Destroy condition variable at module uninit, after all threads are stopped.
+     * xconfProfileLock is statically initialized (PTHREAD_MUTEX_INITIALIZER) and
+     * does not need to be destroyed — destroying it would leave it invalid if
+     * the module were ever re-initialized in the same process. */
     pthread_cond_destroy(&reuseThread);
-    pthread_mutex_destroy(&xconfProfileLock);
     T2Debug("%s --out\n", __FUNCTION__);
     return T2ERROR_SUCCESS;
 }
@@ -724,9 +727,8 @@ T2ERROR ProfileXConf_set(ProfileXConf *profile)
     if(!singleProfile)
     {
         singleProfile = profile;
-        pthread_mutex_lock(&singleProfile->reportInProgressMutex);
-        singleProfile->reportInProgress = false ;
-        pthread_mutex_unlock(&singleProfile->reportInProgressMutex);
+        /* reportInProgress is protected by xconfProfileLock (already held) */
+        singleProfile->reportInProgress = false;
         size_t emIndex = 0;
         EventMarker *eMarker = NULL;
         for(; emIndex < Vector_Size(singleProfile->eMarkerList); emIndex++)
@@ -887,9 +889,8 @@ T2ERROR ProfileXConf_delete(ProfileXConf *profile)
     if(isNameEqual)
     {
         profile->bClearSeekMap = singleProfile->bClearSeekMap ;
-        pthread_mutex_lock(&profile->reportInProgressMutex);
-        profile->reportInProgress = false ;
-        pthread_mutex_unlock(&profile->reportInProgressMutex);
+        /* reportInProgress is protected by xconfProfileLock (already held) */
+        profile->reportInProgress = false;
         if(count > 0 && profile->cachedReportList != NULL)
         {
             T2Info("There are %zu cached reports in the profile \n", count);
@@ -1037,12 +1038,11 @@ void ProfileXConf_notifyTimeout(bool isClearSeekMap, bool isOnDemand)
         return ;
     }
     isOnDemandReport = isOnDemand;
-    pthread_mutex_lock(&singleProfile->reportInProgressMutex);
+    /* reportInProgress is protected by xconfProfileLock (already held) */
     if(!singleProfile->reportInProgress)
     {
         singleProfile->bClearSeekMap = isClearSeekMap;
         singleProfile->reportInProgress = true;
-        pthread_mutex_unlock(&singleProfile->reportInProgressMutex);
 
         if (reportThreadExits)
         {
@@ -1055,11 +1055,20 @@ void ProfileXConf_notifyTimeout(bool isClearSeekMap, bool isOnDemand)
             {
                 T2Error("Failed to create report thread with error code = %d !!! \n", reportThreadStatus);
             }
+            else
+            {
+                /* Set flag immediately so ProfileXConf_uninit can find and
+                 * join this thread.  Without this, there is a race window
+                 * between pthread_create returning and the new thread
+                 * setting reportThreadExits=true inside CollectAndReportXconf,
+                 * during which uninit would skip the join and free resources
+                 * the thread is about to use. */
+                reportThreadExits = true;
+            }
         }
     }
     else
     {
-        pthread_mutex_unlock(&singleProfile->reportInProgressMutex);
         T2Warning("Received profileTimeoutCb while previous callback is still in progress - ignoring the request\n");
     }