Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replay: fix various synchronization and event handling issues #34254

Merged

Conversation

deanlee
Copy link
Contributor

@deanlee deanlee commented Dec 16, 2024

Summary of Changes

  • Thread-safe shared pointer access: Protected std::shared_ptr access using std::atomic_load and std::atomic_store to ensure thread-safe reads and writes.
  • Double-free fix: Resolved a double-free issue occurring during Cabana's Slider::paintEvent.
  • Seek process improvement: Simplified the seeking process and fixed a potential hang when scrubbing by shift-dragging on the chart in Cabana.
  • Stream thread synchronization: Ensured the stream thread doesn’t resume until the onSegmentsMerged callback is fully finished to prevent data inconsistencies.
  • Synchronized slot function: Used a mutex and condition_variable to wait for the queued slot function to finish, ensuring it stays in sync with the replay's handleSegmentMerge thread.
  • Callback outside stream lock: Prevented potential deadlocks by invoking callbacks outside of the stream lock.
  • Code cleanup: Corrected indentation issues in timeline.h for better readability.

@github-actions github-actions bot added the tools label Dec 16, 2024
@deanlee deanlee changed the title replay: refactor to ensure thread-safe access to std::shared_ptr replay: ensure thread-safe access to std::shared_ptr Dec 16, 2024
@deanlee deanlee marked this pull request as draft December 16, 2024 18:27
@deanlee deanlee force-pushed the replay_fix_timeline_race_condition branch from f9607d2 to daac837 Compare December 16, 2024 18:44
@deanlee deanlee marked this pull request as ready for review December 16, 2024 20:19
@deanlee deanlee force-pushed the replay_fix_timeline_race_condition branch 2 times, most recently from 85a2d31 to ab677fa Compare December 17, 2024 18:51
@deanlee deanlee changed the title replay: ensure thread-safe access to std::shared_ptr replay: fix various synchronization and event handling issues Dec 17, 2024
@deanlee deanlee force-pushed the replay_fix_timeline_race_condition branch 8 times, most recently from 44ac2b9 to af26fa5 Compare December 19, 2024 06:44
@deanlee deanlee force-pushed the replay_fix_timeline_race_condition branch from af26fa5 to 2d6b54d Compare December 21, 2024 22:17
@adeebshihadeh adeebshihadeh merged commit d621469 into commaai:master Dec 21, 2024
16 checks passed
@deanlee deanlee deleted the replay_fix_timeline_race_condition branch December 22, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants