Release/1.1.14 - #216
Conversation
There was a problem hiding this comment.
Pull request overview
This release PR updates the RF4CE network initialization wait logic to revert a prior deadlock-mitigation change, and records the release entry in the changelog.
Changes:
- Replaced the RF4CE init semaphore timed wait / EINTR handling with a plain
sem_wait(). - Removed the now-unused
<errno.h>include. - Added the
1.1.14entry toCHANGELOG.mdnoting the revert.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/rf4ce/ctrlm_rf4ce_network.cpp |
Reverts RF4CE init wait behavior back to an unbounded sem_wait() and removes errno-based handling. |
CHANGELOG.md |
Adds the 1.1.14 release notes entry referencing the revert PR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sem_wait(&semaphore_); | ||
| sem_destroy(&semaphore_); |
There was a problem hiding this comment.
sem_wait() return value is ignored. If it returns -1 with EINTR, this code will still call sem_destroy(), and hal_init_confirm() may later sem_post() a destroyed semaphore (undefined behavior / possible crash). Please handle sem_wait() errors (retry on EINTR; otherwise set init_result_ and avoid destroying/using the semaphore incorrectly).
| // 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_); |
There was a problem hiding this comment.
This block no longer enforces a timeout despite the comment saying it does, and sem_wait() can now block indefinitely if the HAL thread never invokes the init confirm callback (e.g., early init failure). Either reintroduce a bounded wait (and propagate a timeout error) or update the init path to guarantee the semaphore is always posted on all failure paths, so ctrlm_main_thread can't deadlock here.
No description provided.