RDKEMW-19034: [support/8.5.3.0] Xconf thread should wait Indefinitely for the NTP indicator#374
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an optional (compile-time) boot-time gate to the xconf worker thread so it can wait for an NTP “sync indicator” file before attempting the initial xconf fetch (to avoid TLS failures when system time isn’t yet valid).
Changes:
- Introduces NTP sync gating helpers (
waitForNTPSyncDir,waitForNTPSync) using inotify + select for low-CPU waiting and interruptible shutdown. - Adds platform-specific paths/macros for the NTP indicator file and its parent directory.
- Hooks the NTP wait into
getUpdatedConfigurationThread()before entering the main fetch loop.
Comments suppressed due to low confidence (1)
source/xconf-client/xconfclient.c:1045
- stopFetchRemoteConfiguration is read under xcThreadMutex here, but it is written elsewhere without consistently taking the same mutex (e.g., stopXConfClient()/uninitXConfClient()). That makes the shutdown check a data race and can cause the new indefinite NTP wait to miss shutdown and hang on pthread_join. Consider making stopFetchRemoteConfiguration an atomic_bool or enforcing a single mutex for all reads/writes.
{
pthread_mutex_lock(&xcThreadMutex);
bool shouldStop = stopFetchRemoteConfiguration;
pthread_mutex_unlock(&xcThreadMutex);
Comment on lines
+1000
to
+1005
| /* | ||
| * Most likely cause: NTP_SYNC_DIR does not exist yet (e.g. /tmp/systimemgr | ||
| * is created by systimemgr at startup). Wait up to 30 minutes for the | ||
| * directory to appear. | ||
| */ | ||
| T2Warning("inotify_add_watch on %s failed with errno=%d, waiting for directory\n", NTP_SYNC_DIR, errno); |
Comment on lines
+919
to
+923
| { | ||
| pthread_mutex_lock(&xcThreadMutex); | ||
| bool shouldStop = stopFetchRemoteConfiguration; | ||
| pthread_mutex_unlock(&xcThreadMutex); | ||
|
|
Comment on lines
+1071
to
+1076
| if (ret == 0) | ||
| { | ||
| continue; /* timeout, loop back to check shutdown flag */ | ||
| } | ||
|
|
||
| ssize_t len = read(ifd, buf, sizeof(buf)); |
Comment on lines
+1120
to
+1125
| #if defined(NTP_SYNC_INDICATION) | ||
| pthread_mutex_unlock(&xcThreadMutex); | ||
| if (!waitForNTPSync()) | ||
| { | ||
| T2Warning("Proceeding without NTP sync confirmation\n"); | ||
| } |
yogeswaransky
changed the title
shibu-kv
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.