diff --git a/CHANGELOG.md b/CHANGELOG.md index 3108098b..a3936713 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,22 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [1.11.0](https://github.com/rdkcentral/telemetry/compare/1.10.4...1.11.0) + +- TCXB7-7251: Fix marker loss on in-place log truncation [`#445`](https://github.com/rdkcentral/telemetry/pull/445) +- 62336: Fix Coverity defects (1 file) [`#433`](https://github.com/rdkcentral/telemetry/pull/433) +- RDKB-62334: Fix Coverity defects (1 file) [`#436`](https://github.com/rdkcentral/telemetry/pull/436) +- 62335 :Fix scheduler profile teardown wait for CID-56336 [`#444`](https://github.com/rdkcentral/telemetry/pull/444) +- RDK-62333: Fix Coverity defects [`#443`](https://github.com/rdkcentral/telemetry/pull/443) +- RDKEMW-test: Fix Coverity defects for RESOURCE_LEAK - CID-51128 and CID-51137 [`#427`](https://github.com/rdkcentral/telemetry/pull/427) +- RDKB-64502: [RDKB][TELEMETRY2_0] Multi‑profile config using method = subscribe, logs null & empty values [`#406`](https://github.com/rdkcentral/telemetry/pull/406) + #### [1.10.4](https://github.com/rdkcentral/telemetry/compare/1.10.3...1.10.4) +> 10 September 2026 + - RDKB-66418: Handle rbus method failure efficiently [`#423`](https://github.com/rdkcentral/telemetry/pull/423) +- Changelog updates for 1.10.4 release [`7a92306`](https://github.com/rdkcentral/telemetry/commit/7a923068c2e863ba96633d8c5b615296c18d560b) #### [1.10.3](https://github.com/rdkcentral/telemetry/compare/1.10.2...1.10.3) diff --git a/source/bulkdata/profile.c b/source/bulkdata/profile.c index ec02fac1..67a2e554 100644 --- a/source/bulkdata/profile.c +++ b/source/bulkdata/profile.c @@ -67,6 +67,19 @@ typedef struct __triggerConditionObj__ char referenceValue[MAX_LEN]; } triggerConditionObj ; +static bool isEmptySubscriberValue(const char *value) +{ + if(value == NULL) + { + return true; + } + if(strlen(value) < 1 || value[0] == ' ' || strncmp(value, "NULL", 4) == 0) + { + return true; + } + return false; +} + static void freeRequestURIparam(void *data) { if(data != NULL) @@ -233,6 +246,10 @@ void freeProfile(void *data) cJSON_Delete(profile->jsonReportObj); profile->jsonReportObj = NULL; } + if(profile->grepSeekProfile) + { + freeGrepSeekProfile(profile->grepSeekProfile); + } free(profile); } T2Debug("%s ++out \n", __FUNCTION__); @@ -969,6 +986,7 @@ reportThreadEnd : pthread_mutex_lock(&profile->reuseThreadMutex); pthread_mutex_lock(&profile->reportInProgressMutex); profile->reportInProgress = false; + profile->reportThreadCreated = false; pthread_cond_signal(&profile->reportInProgressCond); pthread_mutex_unlock(&profile->reportInProgressMutex); pthread_mutex_unlock(&profile->reuseThreadMutex); @@ -1018,7 +1036,20 @@ void NotifyTimeout(const char* profileName, bool isClearSeekMap) else { pthread_mutex_unlock(&profile->reuseThreadMutex); - pthread_create(&profile->reportThread, NULL, CollectAndReport, (void*)profile); + int createStatus = pthread_create(&profile->reportThread, NULL, CollectAndReport, (void*)profile); + pthread_mutex_lock(&profile->reportInProgressMutex); + if(createStatus == 0 && profile->reportInProgress) + { + profile->reportThreadCreated = true; + } + else + { + profile->reportInProgress = false; + profile->reportThreadCreated = false; + pthread_cond_signal(&profile->reportInProgressCond); + T2Error("Failed to create report thread for profile %s: %d\n", profileName, createStatus); + } + pthread_mutex_unlock(&profile->reportInProgressMutex); } } else @@ -1098,12 +1129,19 @@ T2ERROR Profile_storeMarkerEvent(const char *profileName, T2Event *eventInfo) break; case MTYPE_ACCUMULATE: - T2Debug("Marker type is ACCUMULATE Event Value : %s\n", eventInfo->value); + { + const char *safeValue = (eventInfo->value != NULL) ? eventInfo->value : ""; + T2Debug("Marker type is ACCUMULATE Event Value : %s\n", safeValue); + if(!lookupEvent->reportEmptyParam && isEmptySubscriberValue(safeValue)) + { + T2Debug("Skipping empty/null subscribe marker value for %s\n", lookupEvent->markerName); + break; + } arraySize = Vector_Size(lookupEvent->u.accumulatedValues); T2Debug("Current array size : %d \n", arraySize); if( arraySize < MAX_ACCUMULATE) { - Vector_PushBack(lookupEvent->u.accumulatedValues, strdup(eventInfo->value)); + Vector_PushBack(lookupEvent->u.accumulatedValues, strdup(safeValue)); T2Debug("Sucessfully added value into vector New Size : %d\n", ++arraySize); if(lookupEvent->reportTimestampParam == REPORTTIMESTAMP_UNIXEPOCH) { @@ -1137,16 +1175,24 @@ T2ERROR Profile_storeMarkerEvent(const char *profileName, T2Event *eventInfo) T2Warning("Max size of the array has been reached Ignore New Value\n"); } break; + } case MTYPE_ABSOLUTE: default: + { + const char *safeValue = (eventInfo->value != NULL) ? eventInfo->value : ""; + if(!lookupEvent->reportEmptyParam && isEmptySubscriberValue(safeValue)) + { + T2Debug("Skipping empty/null subscribe marker value for %s\n", lookupEvent->markerName); + break; + } if(lookupEvent->u.markerValue) { free(lookupEvent->u.markerValue); lookupEvent->u.markerValue = NULL; } - lookupEvent->u.markerValue = strdup(eventInfo->value); + lookupEvent->u.markerValue = strdup(safeValue); T2Debug("New marker value saved : %s\n", lookupEvent->u.markerValue); if(lookupEvent->reportTimestampParam == REPORTTIMESTAMP_UNIXEPOCH) { @@ -1175,6 +1221,7 @@ T2ERROR Profile_storeMarkerEvent(const char *profileName, T2Event *eventInfo) } break; } + } pthread_mutex_unlock(&profile->eventMutex); } else @@ -1416,13 +1463,6 @@ T2ERROR deleteAllProfiles(bool delFromDisk) * after setting threadExists = false (see CollectAndReport cleanup). */ } - /* 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); - } if(delFromDisk == true) { removeProfileFromDisk(REPORTPROFILES_PERSISTENCE_PATH, tempProfile->name); @@ -1548,11 +1588,6 @@ T2ERROR deleteProfile(const char *profileName) rbusT2ConsumerUnReg(profile->triggerConditionList); } - if(profile->grepSeekProfile) - { - freeGrepSeekProfile(profile->grepSeekProfile); - } - pthread_mutex_destroy(&profile->reportInProgressMutex); pthread_cond_destroy(&profile->reportInProgressCond); @@ -1953,6 +1988,7 @@ T2ERROR appendTriggerCondition (Profile *tempProfile, const char *referenceName, else { T2Warning("%s : referenceName or referenceValue is published as null, ignoring trigger condition \n ", __FUNCTION__); + free(triggerCond); } } @@ -2062,6 +2098,19 @@ T2ERROR triggerReportOnCondtion(const char *referenceName, const char *reference tempProfile->callBackOnReportGenerationComplete = reportGenerationCompleteReceiver; char *tempProfilename = strdup(tempProfile->name); //RDKB-42640 + if(tempProfilename == NULL) + { + T2Error("Failed to allocate profile name while triggering report condition\n"); + if(tempProfile->jsonReportObj != NULL) + { + cJSON_Delete(tempProfile->jsonReportObj); + tempProfile->jsonReportObj = NULL; + } + tempProfile->triggerReportOnCondition = false; + pthread_mutex_unlock(&tempProfile->triggerCondMutex); + pthread_rwlock_unlock(&profileListLock); + return T2ERROR_FAILURE; + } // profileListLock should be unlocked before sending interrupt for report generation T2Debug("%s : Release lock on &profileListLock\n ", __FUNCTION__); @@ -2070,8 +2119,29 @@ T2ERROR triggerReportOnCondtion(const char *referenceName, const char *reference triggerCondition->oprator, triggerCondition->threshold); if(tempProfile->isSchedulerstarted) { - SendInterruptToTimeoutThread(tempProfilename, false); - // triggerCondMutex will be unlocked by CollectAndReport after report generation + T2ERROR sendRet = SendInterruptToTimeoutThread(tempProfilename, false); + if(sendRet != T2ERROR_SUCCESS) + { + bool workerOwnsTrigger = false; + pthread_mutex_lock(&tempProfile->reportInProgressMutex); + workerOwnsTrigger = tempProfile->reportInProgress && tempProfile->reportThreadCreated; + if(!workerOwnsTrigger) + { + tempProfile->triggerReportOnCondition = false; + } + pthread_mutex_unlock(&tempProfile->reportInProgressMutex); + + if(!workerOwnsTrigger) + { + T2Info("For Profile %s SendInterruptToTimeoutThread failed, releasing lock\n", tempProfilename); + pthread_mutex_unlock(&tempProfile->triggerCondMutex); + if(tempProfile->callBackOnReportGenerationComplete) + { + tempProfile->callBackOnReportGenerationComplete(tempProfilename); + } + } + } + // On success: triggerCondMutex will be unlocked by CollectAndReport after report generation } else { @@ -2100,6 +2170,22 @@ T2ERROR triggerReportOnCondtion(const char *referenceName, const char *reference return T2ERROR_SUCCESS; } +#ifdef GTEST_ENABLE +void test_set_profile_list(Vector *testProfileList) +{ + profileList = testProfileList; + initialized = true; + pthread_rwlock_init(&profileListLock, NULL); +} + +void test_reset_profile_list(void) +{ + initialized = false; + profileList = NULL; + pthread_rwlock_destroy(&profileListLock); +} +#endif + unsigned int getMinThresholdDuration(char *profileName) { unsigned int minThresholdDuration = 0; diff --git a/source/bulkdata/profile.h b/source/bulkdata/profile.h index 7bbfaa24..50183af1 100644 --- a/source/bulkdata/profile.h +++ b/source/bulkdata/profile.h @@ -110,6 +110,7 @@ typedef struct _Profile pthread_mutex_t reuseThreadMutex; bool restartRequested; bool threadExists; + bool reportThreadCreated; GrepSeekProfile *grepSeekProfile; // To store GrepConfig } Profile; diff --git a/source/bulkdata/profilexconf.c b/source/bulkdata/profilexconf.c index cda933f9..41abe597 100644 --- a/source/bulkdata/profilexconf.c +++ b/source/bulkdata/profilexconf.c @@ -57,6 +57,19 @@ static bool reportThreadExits = false; static bool isAbortTriggered = false ; static bool isOnDemandReport = false ; +static bool isEmptySubscriberValue(const char *value) +{ + if(value == NULL) + { + return true; + } + if(strlen(value) < 1 || value[0] == ' ' || strncmp(value, "NULL", 4) == 0) + { + return true; + } + return false; +} + #ifdef GTEST_ENABLE #define sendReportOverHTTP __wrap_sendReportOverHTTP #define sendCachedReportsOverHTTP __wrap_sendCachedReportsOverHTTP @@ -100,6 +113,29 @@ static void freeConfig(void *data) } } +static void publishReportUploadStatusForResult(T2ERROR ret) +{ + pthread_mutex_lock(&xconfProfileLock); + bool localIsOnDemandReport = isOnDemandReport; + bool localIsAbortTriggered = isAbortTriggered; + isAbortTriggered = false; + pthread_mutex_unlock(&xconfProfileLock); + + if(!localIsOnDemandReport) + { + return; + } + + if(ret == T2ERROR_FAILURE) + { + publishReportUploadStatus(localIsAbortTriggered ? "ABORTED" : "FAILURE"); + } + else + { + publishReportUploadStatus("SUCCESS"); + } +} + static void freeProfileXConf() { if(singleProfile != NULL) @@ -535,30 +571,7 @@ static void* CollectAndReportXconf(void* data) } // Notify status of upload in case of on demand report upload. - if(isOnDemandReport) - { - if(ret == T2ERROR_FAILURE) - { - if(isAbortTriggered) - { - publishReportUploadStatus("ABORTED"); - } - else - { - publishReportUploadStatus("FAILURE"); - } - } - else - { - publishReportUploadStatus("SUCCESS"); - } - } - - // Reset the abort trigger flags - if(isAbortTriggered == true) - { - isAbortTriggered = false ; - } + publishReportUploadStatusForResult(ret); /* CRITICAL SECTION START: Re-acquire xconfProfileLock before updating profile state. * pthread_cond_wait requires us to hold xconfProfileLock, so we acquire it here @@ -1112,12 +1125,19 @@ T2ERROR ProfileXConf_storeMarkerEvent(T2Event *eventInfo) break; case MTYPE_XCONF_ACCUMULATE: - T2Debug("Marker type is ACCUMULATE Event Value : %s\n", eventInfo->value); + { + const char *safeValue = (eventInfo->value != NULL) ? eventInfo->value : ""; + T2Debug("Marker type is ACCUMULATE Event Value : %s\n", safeValue); + if(!lookupEvent->reportEmptyParam && isEmptySubscriberValue(safeValue)) + { + T2Debug("Skipping empty/null subscribe marker value for %s\n", lookupEvent->markerName); + break; + } arraySize = Vector_Size(lookupEvent->u.accumulatedValues); T2Debug("Current array size : %d \n", arraySize); if( arraySize < MAX_ACCUMULATE) { - Vector_PushBack(lookupEvent->u.accumulatedValues, strdup(eventInfo->value)); + Vector_PushBack(lookupEvent->u.accumulatedValues, strdup(safeValue)); T2Debug("Sucessfully added value into vector New Size : %d\n", ++arraySize); } else if ( arraySize == MAX_ACCUMULATE ) @@ -1131,17 +1151,26 @@ T2ERROR ProfileXConf_storeMarkerEvent(T2Event *eventInfo) T2Warning("Max size of the array has been reached Ignore New Value\n"); } break; + } case MTYPE_XCONF_ABSOLUTE: default: + { + const char *safeValue = (eventInfo->value != NULL) ? eventInfo->value : ""; + if(!lookupEvent->reportEmptyParam && isEmptySubscriberValue(safeValue)) + { + T2Debug("Skipping empty/null subscribe marker value for %s\n", lookupEvent->markerName); + break; + } if(lookupEvent->u.markerValue) { free(lookupEvent->u.markerValue); } - lookupEvent->u.markerValue = strdup(eventInfo->value); + lookupEvent->u.markerValue = strdup(safeValue); T2Debug("New marker value saved : %s\n", lookupEvent->u.markerValue); break; } + } } else { @@ -1157,6 +1186,19 @@ T2ERROR ProfileXConf_storeMarkerEvent(T2Event *eventInfo) return T2ERROR_SUCCESS; } #ifdef GTEST_ENABLE +void test_setReportUploadStatusState(bool isOnDemand, bool isAbort) +{ + pthread_mutex_lock(&xconfProfileLock); + isOnDemandReport = isOnDemand; + isAbortTriggered = isAbort; + pthread_mutex_unlock(&xconfProfileLock); +} + +void test_publishReportUploadStatusForResult(T2ERROR ret) +{ + publishReportUploadStatusForResult(ret); +} + void test_set_reportThreadExits(bool value) { reportThreadExits = value; diff --git a/source/bulkdata/reportprofiles.c b/source/bulkdata/reportprofiles.c index 6c02b0b3..080b4197 100644 --- a/source/bulkdata/reportprofiles.c +++ b/source/bulkdata/reportprofiles.c @@ -1048,6 +1048,10 @@ void ReportProfiles_ProcessReportProfilesBlob(cJSON *profiles_root, bool rprofil rm_flag = true; } } + else + { + freeProfile(profile); + } } else { diff --git a/source/bulkdata/t2eventreceiver.c b/source/bulkdata/t2eventreceiver.c index 32a2d019..426dfc2a 100644 --- a/source/bulkdata/t2eventreceiver.c +++ b/source/bulkdata/t2eventreceiver.c @@ -268,6 +268,11 @@ void* T2ER_EventDispatchThread(void *arg) if(ret != 0) // pthread cond wait failed return after unlock { T2Error("%s pthread_cond_wait failed with error code: %d\n", __FUNCTION__, ret); + if(pthread_mutex_unlock(&erMutex) != 0) + { + T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); + } + return NULL; } T2Debug("Received signal from T2ER_Push\n"); // Release erMutex before acquiring sTDMutex to avoid lock order reversal and potential deadlock diff --git a/source/dcautil/dca.c b/source/dcautil/dca.c index 498258d8..cdd07dd2 100644 --- a/source/dcautil/dca.c +++ b/source/dcautil/dca.c @@ -1055,6 +1055,9 @@ static FileDescriptor* getFileDeltaInMemMapAndSearch(const int fd, const off_t s off_t offset_in_page_size_multiple ; unsigned int bytes_ignored = 0, bytes_ignored_main = 0, bytes_ignored_rotated = 0; off_t main_fsize = 0, rotated_fsize = 0; + // Log upload backs up and truncates in place, so the stored seek no longer refers + // to any data still present in this file. + const bool truncated = (seek_value > sb.st_size); // Find the nearest multiple of page size if (seek_value > 0 && PAGESIZE > 0) { @@ -1186,9 +1189,19 @@ static FileDescriptor* getFileDeltaInMemMapAndSearch(const int fd, const off_t s // No inode change (seek_value > sb.st_size or check_rotated path) // Legacy behavior: read entire rotated + current from seek rotated_fsize = rb.st_size; - main_fsize = sb.st_size - seek_value; - bytes_ignored_main = bytes_ignored; - addrcf = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, tmp_fd, offset_in_page_size_multiple); + if(truncated) + { + // Stale seek points past the truncated file - read all of it + main_fsize = sb.st_size; + bytes_ignored_main = 0; + addrcf = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, tmp_fd, 0); + } + else + { + main_fsize = sb.st_size - seek_value; + bytes_ignored_main = bytes_ignored; + addrcf = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, tmp_fd, offset_in_page_size_multiple); + } addrrf = mmap(NULL, rb.st_size, PROT_READ, MAP_PRIVATE, tmp_rd, 0); } if(tmp_rd != -1) @@ -1216,6 +1229,16 @@ static FileDescriptor* getFileDeltaInMemMapAndSearch(const int fd, const off_t s bytes_ignored_main = bytes_ignored; main_fsize = sb.st_size - seek_value; } + else if(truncated) + { + // Truncated in place with no rotated file to fall back on - read the + // entire current file so its markers are still collected this cycle + T2Info("Log file %s truncated (seek %jd > size %jd), reading entire current file\n", + logFile, (intmax_t)seek_value, (intmax_t)sb.st_size); + addrcf = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, tmp_fd, 0); + bytes_ignored_main = 0; + main_fsize = sb.st_size; + } else { diff --git a/source/reportgen/reportgen.c b/source/reportgen/reportgen.c index 4b5c325a..98834b1f 100644 --- a/source/reportgen/reportgen.c +++ b/source/reportgen/reportgen.c @@ -1567,6 +1567,13 @@ T2ERROR encodeEventMarkersInJSON(cJSON *valArray, Vector *eventMarkerList) default: if(eventMarker->u.markerValue != NULL) { + if(!eventMarker->reportEmptyParam && checkForEmptyString(eventMarker->u.markerValue)) + { + T2Debug("Skipping empty/\"NULL\" marker value for : %s\n", eventMarker->markerName); + free(eventMarker->u.markerValue); + eventMarker->u.markerValue = NULL; + break; + } arrayItem = cJSON_CreateObject(); if(arrayItem == NULL) { diff --git a/source/scheduler/scheduler.c b/source/scheduler/scheduler.c index f6a73710..537c98d7 100644 --- a/source/scheduler/scheduler.c +++ b/source/scheduler/scheduler.c @@ -70,12 +70,33 @@ void freeSchedulerProfile(void *data) pthread_mutex_destroy(&schProfile->tMutex); pthread_cond_destroy(&schProfile->tCond); pthread_detach(schProfile->tId); + pthread_mutex_destroy(&schProfile->terminationMutex); + pthread_cond_destroy(&schProfile->terminationCond); free(schProfile->name); schProfile->name = NULL; free(schProfile); } } +static void *schedulerThreadExit(SchedulerProfile *tProfile) +{ + bool removeOnExit = false; + + pthread_mutex_lock(&tProfile->terminationMutex); + tProfile->threadExited = true; + removeOnExit = tProfile->removeOnThreadExit; + pthread_cond_broadcast(&tProfile->terminationCond); + pthread_mutex_unlock(&tProfile->terminationMutex); + + if(removeOnExit) + { + pthread_mutex_lock(&scMutex); + Vector_RemoveItem(profileList, tProfile, freeSchedulerProfile); + pthread_mutex_unlock(&scMutex); + } + return NULL; +} + /* * Function: getLapsedTime * -------------------- @@ -169,7 +190,7 @@ void* TimeoutThread(void *arg) if (pthread_condattr_init(&Profile_attr) != 0) { T2Error("pthread_condattr_init failed"); - return NULL; + return schedulerThreadExit(tProfile); } //Set the clock source for the condition variable as CLOCK_MONOTONIC @@ -181,7 +202,7 @@ void* TimeoutThread(void *arg) { T2Error("pthread_condattr_destroy failed \n"); } - return NULL; + return schedulerThreadExit(tProfile); } //Initialize the condition variable with the attributes @@ -192,7 +213,7 @@ void* TimeoutThread(void *arg) { T2Error("pthread_condattr_destroy failed \n"); } - return NULL; + return schedulerThreadExit(tProfile); } if (pthread_condattr_destroy(&Profile_attr) != 0) @@ -208,7 +229,7 @@ void* TimeoutThread(void *arg) if(pthread_mutex_lock(&tProfile->tMutex) != 0) { T2Error("tProfile Mutex lock failed\n"); - return NULL; + return schedulerThreadExit(tProfile); } // Check loop conditions while holding the lock @@ -286,7 +307,7 @@ void* TimeoutThread(void *arg) if(pthread_mutex_lock(&tProfile->tMutex) != 0) { T2Error("tProfile Mutex lock failed after notifySchedulerstartcb\n"); - return NULL; + return schedulerThreadExit(tProfile); } //When first reporting interval is given waiting for first report int vale if(tProfile->firstreportint > 0 && tProfile->firstexecution == true ) @@ -352,7 +373,7 @@ void* TimeoutThread(void *arg) if(pthread_mutex_unlock(&tProfile->tMutex) != 0) { T2Error("tProfile Mutex unlock failed\n"); - return NULL; + return schedulerThreadExit(tProfile); } break; } @@ -395,7 +416,7 @@ void* TimeoutThread(void *arg) { T2Error("tProfile Mutex unlock failed\n"); free(profileName); - return NULL; + return schedulerThreadExit(tProfile); } activationTimeoutCb(profileName); is_activation_time_out = false; @@ -408,11 +429,11 @@ void* TimeoutThread(void *arg) if(pthread_mutex_unlock(&tProfile->tMutex) != 0) { T2Error("tProfile Mutex unlock failed\n"); - return NULL; + return schedulerThreadExit(tProfile); } } T2Debug("%s --out\n", __FUNCTION__); - return NULL; + return schedulerThreadExit(tProfile); } T2ERROR SendInterruptToTimeoutThread(char* profileName, bool isClearSeekMap) @@ -440,6 +461,11 @@ T2ERROR SendInterruptToTimeoutThread(char* profileName, bool isClearSeekMap) tProfile = (SchedulerProfile *)Vector_At(profileList, index); if(profileName == NULL || (strcmp(profileName, tProfile->name) == 0)) { + if(tProfile->removing) + { + pthread_mutex_unlock(&scMutex); + return T2ERROR_FAILURE; + } T2Info("Sending Interrupt signal to Timeout Thread of profile : %s\n", tProfile->name); int mutex_return = pthread_mutex_trylock(&tProfile->tMutex); if(mutex_return != 0) @@ -596,7 +622,18 @@ T2ERROR registerProfileWithScheduler(const char* profileName, unsigned int timeI } SchedulerProfile *tProfile = (SchedulerProfile *)malloc(sizeof(SchedulerProfile)); + if(tProfile == NULL) + { + T2Error("Failed to allocate scheduler profile\n"); + return T2ERROR_FAILURE; + } tProfile->name = strdup(profileName); + if(tProfile->name == NULL) + { + T2Error("Failed to allocate scheduler profile name\n"); + free(tProfile); + return T2ERROR_FAILURE; + } tProfile->repeat = repeat; tProfile->timeOutDuration = timeInterval; tProfile->timeToLive = activationTimeout; @@ -606,6 +643,9 @@ T2ERROR registerProfileWithScheduler(const char* profileName, unsigned int timeI tProfile->firstreportint = firstReportingInterval; tProfile->firstexecution = false; tProfile->isClearSeekMap = false; + tProfile->removing = false; + tProfile->removeOnThreadExit = false; + tProfile->threadExited = false; tProfile->timeRef = timeRef; tProfile->timeRefinSec = 0; if(tProfile->timeOutDuration < tProfile->firstreportint) @@ -619,6 +659,25 @@ T2ERROR registerProfileWithScheduler(const char* profileName, unsigned int timeI if(pthread_mutex_init(&tProfile->tMutex, NULL) != 0) { T2Error("%s Mutex init has failed\n", __FUNCTION__); + free(tProfile->name); + free(tProfile); + return T2ERROR_FAILURE; + } + if(pthread_mutex_init(&tProfile->terminationMutex, NULL) != 0) + { + T2Error("%s termination mutex init has failed\n", __FUNCTION__); + pthread_mutex_destroy(&tProfile->tMutex); + free(tProfile->name); + free(tProfile); + return T2ERROR_FAILURE; + } + if(pthread_cond_init(&tProfile->terminationCond, NULL) != 0) + { + T2Error("%s termination condition init has failed\n", __FUNCTION__); + pthread_mutex_destroy(&tProfile->terminationMutex); + pthread_mutex_destroy(&tProfile->tMutex); + free(tProfile->name); + free(tProfile); return T2ERROR_FAILURE; } pthread_cond_init(&tProfile->tCond, NULL); @@ -672,6 +731,11 @@ T2ERROR unregisterProfileFromScheduler(const char* profileName) if(strcmp(tProfile->name, profileName) == 0) { + if(tProfile->removing) + { + pthread_mutex_unlock(&scMutex); + return T2ERROR_FAILURE; + } if(pthread_mutex_lock(&tProfile->tMutex) != 0) { T2Error("tProfile Mutex lock failed\n"); @@ -679,6 +743,7 @@ T2ERROR unregisterProfileFromScheduler(const char* profileName) return T2ERROR_FAILURE; } tProfile->terminated = true; + tProfile->removing = true; signalrecived_and_executing = true; pthread_cond_signal(&tProfile->tCond); if(pthread_mutex_unlock(&tProfile->tMutex) != 0) @@ -688,37 +753,25 @@ T2ERROR unregisterProfileFromScheduler(const char* profileName) return T2ERROR_FAILURE; } T2Info(" tProfile->tId = %d tProfile->name = %s\n", (int)tProfile->tId, tProfile->name); - // pthread_join(tProfile->tId, NULL); // pthread_detach in freeSchedulerProfile will detach the thread - sched_yield(); // This will give chance for the signal receiving thread to start - int count = 0; - bool is_signal_executing = true; - while(is_signal_executing && !is_activation_time_out) + if(pthread_equal(pthread_self(), tProfile->tId)) { - if(pthread_mutex_lock(&tProfile->tMutex) != 0) - { - T2Error("tProfile Mutex lock failed\n"); - pthread_mutex_unlock(&scMutex); - return T2ERROR_FAILURE; - } - is_signal_executing = signalrecived_and_executing; - if(pthread_mutex_unlock(&tProfile->tMutex) != 0) - { - T2Error("tProfile Mutex unlock failed\n"); - pthread_mutex_unlock(&scMutex); - return T2ERROR_FAILURE; - } - if(count++ > 10) - { - break; - } - sleep(1); + pthread_mutex_lock(&tProfile->terminationMutex); + tProfile->removeOnThreadExit = true; + pthread_mutex_unlock(&tProfile->terminationMutex); + pthread_mutex_unlock(&scMutex); + return T2ERROR_SUCCESS; + } + + pthread_mutex_unlock(&scMutex); + pthread_mutex_lock(&tProfile->terminationMutex); + while(!tProfile->threadExited) + { + pthread_cond_wait(&tProfile->terminationCond, &tProfile->terminationMutex); } + pthread_mutex_unlock(&tProfile->terminationMutex); - // Keep scMutex held across the wait loop to prevent concurrent removal/free of tProfile. - // This avoids using a potentially stale/freed pointer (Coverity ATOMICITY/CWE-662). - // Don't lock tProfile->tMutex here as Vector_RemoveItem will free tProfile via - // freeSchedulerProfile callback, which would cause use-after-free when unlocking. + pthread_mutex_lock(&scMutex); Vector_RemoveItem(profileList, tProfile, freeSchedulerProfile); T2Debug("%s:%d scMutex is unlocked\n", __FUNCTION__, __LINE__); diff --git a/source/scheduler/scheduler.h b/source/scheduler/scheduler.h index a1d6c885..8f6dba7a 100644 --- a/source/scheduler/scheduler.h +++ b/source/scheduler/scheduler.h @@ -39,9 +39,14 @@ typedef struct _SchedulerProfile unsigned int firstreportint; bool firstexecution; bool isClearSeekMap; + bool removing; + bool removeOnThreadExit; + bool threadExited; pthread_t tId; pthread_mutex_t tMutex; pthread_cond_t tCond; + pthread_mutex_t terminationMutex; + pthread_cond_t terminationCond; } SchedulerProfile; diff --git a/source/test/bulkdata/profileTest.cpp b/source/test/bulkdata/profileTest.cpp index f56d05f3..f76e377c 100644 --- a/source/test/bulkdata/profileTest.cpp +++ b/source/test/bulkdata/profileTest.cpp @@ -39,6 +39,8 @@ extern "C" { #include extern bool initialized; +void test_set_profile_list(Vector *testProfileList); +void test_reset_profile_list(void); sigset_t blocking_signal; hash_map_t *markerCompMap = NULL; @@ -89,6 +91,20 @@ class ProfileTest : public ::testing::Test { //comment //==================================== profile.c =================== +TEST_F(ProfileTest, FreeProfile_CleansUpGrepSeekProfile) { + Profile *profile = (Profile *)calloc(1, sizeof(Profile)); + ASSERT_NE(profile, nullptr); + + profile->grepSeekProfile = createGrepSeekProfile(0); + if (profile->grepSeekProfile == nullptr) + { + freeProfile(profile); + FAIL() << "createGrepSeekProfile(0) failed"; + } + + freeProfile(profile); +} + // Test initProfileList TEST_F(ProfileTest, InitProfileList_Success) { const char* path = "/tmp/t2reportprofiles/"; @@ -302,6 +318,59 @@ TEST_F(ProfileTest, triggerReportOnCondtion) { EXPECT_EQ(triggerReportOnCondtion("refname", "refvalue"), T2ERROR_SUCCESS); } +TEST_F(ProfileTest, TriggerReportOnCondition_SchedulerFailureKeepsActiveMutexOwner) { + delete g_vectorMock; + g_vectorMock = nullptr; + + Vector *profiles = nullptr; + Vector *triggerConditions = nullptr; + Vector_Create(&profiles); + Vector_Create(&triggerConditions); + + Profile *profile = (Profile *)calloc(1, sizeof(Profile)); + TriggerCondition *condition = (TriggerCondition *)calloc(1, sizeof(TriggerCondition)); + ASSERT_NE(profile, nullptr); + ASSERT_NE(condition, nullptr); + + profile->name = strdup("FailureProfile"); + profile->isSchedulerstarted = true; + profile->reportInProgress = true; + profile->reportThreadCreated = true; + profile->triggerConditionList = triggerConditions; + pthread_mutex_init(&profile->triggerCondMutex, nullptr); + pthread_mutex_init(&profile->reportInProgressMutex, nullptr); + condition->reference = strdup("refname"); + condition->report = true; + Vector_PushBack(triggerConditions, condition); + Vector_PushBack(profiles, profile); + test_set_profile_list(profiles); + + EXPECT_CALL(*g_schedulerMock, SendInterruptToTimeoutThread(_, false)) + .Times(2) + .WillRepeatedly(Return(T2ERROR_FAILURE)); + + EXPECT_EQ(triggerReportOnCondtion("refname", "refvalue"), T2ERROR_SUCCESS); + EXPECT_TRUE(profile->triggerReportOnCondition); + EXPECT_NE(pthread_mutex_trylock(&profile->triggerCondMutex), 0); + + pthread_mutex_unlock(&profile->triggerCondMutex); + profile->reportInProgress = false; + profile->reportThreadCreated = false; + cJSON_Delete(profile->jsonReportObj); + profile->jsonReportObj = nullptr; + + EXPECT_EQ(triggerReportOnCondtion("refname", "refvalue"), T2ERROR_SUCCESS); + EXPECT_FALSE(profile->triggerReportOnCondition); + EXPECT_EQ(pthread_mutex_trylock(&profile->triggerCondMutex), 0); + pthread_mutex_unlock(&profile->triggerCondMutex); + + pthread_mutex_destroy(&profile->triggerCondMutex); + pthread_mutex_destroy(&profile->reportInProgressMutex); + freeProfile(profile); + Vector_Destroy(profiles, nullptr); + test_reset_profile_list(); +} + TEST_F(ProfileTest, getMinThresholdDuration_Failure) { EXPECT_CALL(*g_vectorMock, Vector_Size(_)).Times(::testing::AtMost(1)).WillRepeatedly(Return(0)); EXPECT_EQ(getMinThresholdDuration("profile1"), 0); diff --git a/source/test/bulkdata/profilexconfTest.cpp b/source/test/bulkdata/profilexconfTest.cpp index 7ab28d2d..e2ad1068 100644 --- a/source/test/bulkdata/profilexconfTest.cpp +++ b/source/test/bulkdata/profilexconfTest.cpp @@ -55,6 +55,9 @@ using ::testing::Invoke; using ::testing::SetArgPointee; using ::testing::DoAll; +extern "C" void test_setReportUploadStatusState(bool isOnDemand, bool isAbort); +extern "C" void test_publishReportUploadStatusForResult(T2ERROR ret); + rbusMock* g_rbusMock = nullptr; SystemMock* g_systemMock = nullptr; FileMock* g_fileIOMock = nullptr; @@ -548,6 +551,29 @@ TEST_F(profileXconfTestFixture, ReportProfiles_storeMarkerEvent_success_2) free(eventInfo); } +TEST_F(profileXconfTestFixture, ProfileXConf_OnDemandStatusSnapshot) +{ + testing::InSequence sequence; + EXPECT_CALL(*g_profileXConfMock, publishReportUploadStatus(StrEq("ABORTED"))) + .Times(1); + EXPECT_CALL(*g_profileXConfMock, publishReportUploadStatus(StrEq("FAILURE"))) + .Times(1); + + test_setReportUploadStatusState(true, true); + test_publishReportUploadStatusForResult(T2ERROR_FAILURE); + test_publishReportUploadStatusForResult(T2ERROR_FAILURE); + test_setReportUploadStatusState(false, false); +} + +TEST_F(profileXconfTestFixture, ProfileXConf_NonOnDemandStatusIsIgnored) +{ + EXPECT_CALL(*g_profileXConfMock, publishReportUploadStatus(_)) + .Times(0); + + test_setReportUploadStatusState(false, true); + test_publishReportUploadStatusForResult(T2ERROR_FAILURE); +} + //ProfileXConf_notifyTimeout Test the timeout of the profile /* TEST_F(profileXconfTestFixture, ProfileXConf_notifyTimeout) diff --git a/source/test/dcautils/dcautilTest.cpp b/source/test/dcautils/dcautilTest.cpp index a8113dff..f54077ab 100644 --- a/source/test/dcautils/dcautilTest.cpp +++ b/source/test/dcautils/dcautilTest.cpp @@ -1771,6 +1771,91 @@ TEST_F(dcaTestFixture, getDCAResultsInVector_MultiRotation_IntermediateFile) Vector_Destroy(vecMarkerList, freeGMarker); } +// TCXB7-7251: PARODUSlog.txt truncated in place by log upload with no rotated +// backup available - stale seek must not cause the marker to be dropped. +TEST_F(dcaTestFixture, getDCAResultsInVector_TruncatedInPlace_NoRotatedFile) +{ + GrepSeekProfile *gsProfile = (GrepSeekProfile *)malloc(sizeof(GrepSeekProfile)); + gsProfile->logFileSeekMap = hash_map_create(); + gsProfile->execCounter = 2; + long *tempnum; + double val = 5000; + tempnum = (long *)malloc(sizeof(long)); + *tempnum = (long)val; + hash_map_put(gsProfile->logFileSeekMap, strdup("PARODUSlog.txt"), (void*)tempnum, free); + + Vector* vecMarkerList = NULL; + Vector_Create(&vecMarkerList); + GrepMarker* marker = (GrepMarker*) malloc(sizeof(GrepMarker)); + memset(marker, 0, sizeof(GrepMarker)); + marker->markerName = strdup("SYS_INFO_LowQosEvntUpload"); + marker->searchString = strdup("PARODUS: Low qos event"); + marker->trimParam = true; + marker->regexParam = strdup("[0-9]+"); + marker->logFile = strdup("PARODUSlog.txt"); + marker->skipFreq = 0; + marker->paramType = strdup("grep"); + marker->mType = MTYPE_COUNTER; + marker->u.count = 0; + marker->reportEmptyParam = true; + Vector_PushBack(vecMarkerList, (void*) marker); + + //freeFileDescriptor + EXPECT_CALL(*g_fileIOMock, munmap(_, _)) + .Times(1) + .WillOnce(Return(0)); + EXPECT_CALL(*g_fileIOMock, close(_)) + .Times(2) + .WillOnce(Return(0)) + .WillOnce(Return(0)); + + //getLogFileDescriptor opens the current (truncated) file, then + //getRotatedLogFileDescriptor fails to open the missing .1 file + EXPECT_CALL(*g_fileIOMock, open(_,_)) + .Times(2) + .WillOnce(Return(0)) + .WillOnce(Return(-1)); + EXPECT_CALL(*g_fileIOMock, fstat(_, _)) + .Times(2) + .WillOnce([](int fd, struct stat* statbuf) { + statbuf->st_size = 800; // Truncated file size, smaller than stored seek 5000 + return 0; + }) + .WillOnce([](int fd, struct stat* statbuf) { + statbuf->st_size = 800; + return 0; + }); + + //getDeltainmmapsearch - only the current file is staged, no rotated tmp file + EXPECT_CALL(*g_fileIOMock, mkstemp(_)) + .Times(1) + .WillOnce(Return(0)); + EXPECT_CALL(*g_systemMock, unlink(_)) + .Times(1) + .WillOnce(Return(0)); + EXPECT_CALL(*g_fileIOMock,sendfile(_,_,_,_)) + .Times(1) + .WillOnce(Return(800)); + EXPECT_CALL(*g_fileIOMock, mmap(_,_,_,_,_,_)) + .Times(1) + .WillOnce([](void *addr, size_t length, int prot, int flags, int fd, off_t offset) { + EXPECT_EQ(0, offset); // Entire current file read from offset 0 despite stale seek + const char* test_str = "PARODUS: Low qos event, send success callback and delete\n"; + char* mapped_mem = (char*)malloc(length); + memset(mapped_mem, 0, length); + strncpy(mapped_mem, test_str, length - 1); + return (void*)mapped_mem; + }); + + EXPECT_EQ(0, getDCAResultsInVector(gsProfile, vecMarkerList, false, "/opt/logs")); + EXPECT_EQ(1, marker->u.count); // Marker must still be captured despite the truncation + + hash_map_destroy(gsProfile->logFileSeekMap, free); + gsProfile->logFileSeekMap = NULL; + free(gsProfile); + Vector_Destroy(vecMarkerList, freeGMarker); +} + TEST_F(dcaTestFixture, T2InitProperties) { EXPECT_CALL(*g_fileIOMock, fopen(_,_)) diff --git a/source/test/scheduler/schedulerTest.cpp b/source/test/scheduler/schedulerTest.cpp index 4186c16a..e1561a96 100644 --- a/source/test/scheduler/schedulerTest.cpp +++ b/source/test/scheduler/schedulerTest.cpp @@ -323,8 +323,14 @@ TEST(TIMEOUTTHREAD, TEST1) tProfile->reportonupdate = true; tProfile->firstreportint = 20; tProfile->firstexecution = true; + tProfile->removeOnThreadExit = false; + tProfile->threadExited = false; + pthread_mutex_init(&tProfile->terminationMutex, NULL); + pthread_cond_init(&tProfile->terminationCond, NULL); TimeoutThread((void *)tProfile); + pthread_mutex_destroy(&tProfile->terminationMutex); + pthread_cond_destroy(&tProfile->terminationCond); free(tProfile->name); free(tProfile); } @@ -342,8 +348,14 @@ TEST(TIMEOUTTHREAD, TEST2) tProfile->reportonupdate = true; tProfile->firstreportint = 20; tProfile->firstexecution = false; + tProfile->removeOnThreadExit = false; + tProfile->threadExited = false; + pthread_mutex_init(&tProfile->terminationMutex, NULL); + pthread_cond_init(&tProfile->terminationCond, NULL); TimeoutThread((void *)tProfile); + pthread_mutex_destroy(&tProfile->terminationMutex); + pthread_cond_destroy(&tProfile->terminationCond); free(tProfile->name); free(tProfile); } @@ -362,10 +374,16 @@ TEST(TIMEOUTTHREAD, WAIT_NO_REPORTING_INTERVAL) tProfile->reportonupdate = true; tProfile->firstreportint = 0; tProfile->firstexecution = false; + tProfile->removeOnThreadExit = false; + tProfile->threadExited = false; + pthread_mutex_init(&tProfile->terminationMutex, NULL); + pthread_cond_init(&tProfile->terminationCond, NULL); // Call TimeoutThread directly – will exercise pthread_cond_wait branch TimeoutThread((void *)tProfile); + pthread_mutex_destroy(&tProfile->terminationMutex); + pthread_cond_destroy(&tProfile->terminationCond); free(tProfile->name); free(tProfile); } @@ -414,6 +432,8 @@ TEST(FREE_SCHEDULER_PROFILE, NORMAL) sch->name = strdup("FREE_PROFILE"); pthread_mutex_init(&sch->tMutex, NULL); pthread_cond_init(&sch->tCond, NULL); + pthread_mutex_init(&sch->terminationMutex, NULL); + pthread_cond_init(&sch->terminationCond, NULL); pthread_t threadId; ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL)); freeSchedulerProfile(sch);