Fix access violation bug - #14173
Conversation
|
Please check GHA CI failures |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an access violation bug by properly managing the lifetime of an initialization thread in the platform_camera class. The issue was caused by a detached thread that could potentially access destroyed object members after the platform_camera object was destroyed.
- Replaces detached thread with a managed thread that can be properly joined on destruction
- Adds early termination capability for the initialization process using an atomic flag
- Refactors option registration from individual calls to a loop-based approach
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/platform-camera.h | Adds destructor declaration, thread member variable, and atomic stop flag |
| src/platform-camera.cpp | Implements proper thread management and early termination logic in initialization |
Comments suppressed due to low confidence (1)
src/platform-camera.h:30
- [nitpick] The variable name 'should_stop' uses snake_case which is inconsistent with the existing codebase style. Consider renaming to '_should_stop' or 'shouldStop' to match the naming convention used for '_init_thread'.
std::atomic< bool > should_stop{ false }; // to avoid delay when closing and _init_thread is still running
| RS2_OPTION_WHITE_BALANCE, | ||
| RS2_OPTION_ENABLE_AUTO_EXPOSURE, | ||
| RS2_OPTION_ENABLE_AUTO_WHITE_BALANCE }; | ||
| for (auto option : options) |
There was a problem hiding this comment.
[nitpick] The loop variable 'option' should be passed by const reference to avoid unnecessary copying of the enum value. Change to 'for (const auto& option : options)' or 'for (auto option : options)' is fine since rs2_option is likely a simple enum, but consistency with modern C++ practices suggests using const auto&.
| for (auto option : options) | |
| for (const auto& option : options) |
There was a problem hiding this comment.
@AviaAv why not taking co pilot advise here? seems like a valid comment
Tracked on: [LRS-1316]