RDKEMW-17520: ctrlm release 1.1.14 - #210
Conversation
There was a problem hiding this comment.
Pull request overview
Release-focused PR for ctrlm 1.1.14 that updates the changelog and reverts the RF4CE HAL init wait logic in ctrlm_rf4ce_network.cpp.
Changes:
- Replaced the HAL init semaphore timed wait/error handling with an unbounded
sem_wait()and removed<errno.h>include. - Added
1.1.14release entry toCHANGELOG.mdnoting the revert.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/rf4ce/ctrlm_rf4ce_network.cpp |
Reverts HAL init wait back to blocking semaphore wait. |
CHANGELOG.md |
Adds 1.1.14 changelog entry referencing the revert. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Block until initialization is complete or a timeout occurs | ||
| XLOGD_INFO("Waiting for %s initialization...", name_get()); | ||
| struct timespec timeout; | ||
| clock_gettime(CLOCK_REALTIME, &timeout); | ||
| timeout.tv_sec += 2; // this operation should complete in under 100 ms under normal circumstances | ||
|
|
||
| int sem_result = -1; | ||
| do { | ||
| errno = 0; | ||
| sem_result = sem_timedwait(&semaphore_, &timeout); | ||
| if(sem_result == -1 && errno == EINTR) { | ||
| XLOGD_INFO("interrupted"); | ||
| } else { | ||
| break; | ||
| } | ||
| } while(1); | ||
|
|
||
| if(sem_result == -1) { | ||
| if(errno == ETIMEDOUT) { | ||
| XLOGD_ERROR("Timeout waiting for %s initialization", name_get()); | ||
| } else { | ||
| XLOGD_ERROR("Error waiting for %s initialization: %s", name_get(), strerror(errno)); | ||
| } | ||
| init_result_ = CTRLM_HAL_RESULT_ERROR; | ||
| } else { | ||
| sem_destroy(&semaphore_); | ||
| } | ||
| sem_wait(&semaphore_); | ||
| sem_destroy(&semaphore_); |
There was a problem hiding this comment.
sem_wait() here can block indefinitely if the HAL thread never calls hal_init_confirm() (e.g., init failure, thread creation failure, or missed callback), reintroducing the deadlock scenario this code previously guarded against. Also sem_wait() return value isn’t checked/handled (e.g., EINTR), yet sem_destroy() is called unconditionally. Consider restoring a bounded wait (sem_timedwait with a reasonable timeout + logging) and handling EINTR/other errors, or otherwise guarantee sem_post() occurs on every init path before destroying the semaphore.
No description provided.