Skip to content

Fix race conditions in TaskContext and CancellationContext cancellation - #1689

Open
diachenko-mischa wants to merge 1 commit into
masterfrom
mdiachenko/HERESDK-12253-PreExecutionCancel
Open

Fix race conditions in TaskContext and CancellationContext cancellation#1689
diachenko-mischa wants to merge 1 commit into
masterfrom
mdiachenko/HERESDK-12253-PreExecutionCancel

Conversation

@diachenko-mischa

Copy link
Copy Markdown
Contributor
  • Enable pre-execution cancellation in TaskContext by registering a cancellation token in constructor that invokes PreExecuteCancel()
  • Clear cancellation token in ~TaskContextImpl to prevent dangling refs
  • Fix CancellationContext::CancelOperation() to swap out the token before calling Cancel(), avoiding re-entrancy issues
  • Update BlockingCancel test to cover pre-execution cancellation

Relates-To: HERESDK-12253

@diachenko-mischa
diachenko-mischaforce-pushed the mdiachenko/HERESDK-12253-PreExecutionCancel branch 3 times, most recently from 105bbc8 to d1cb9e7CompareApril 1, 2026 17:49
@diachenko-mischa
diachenko-mischaforce-pushed the mdiachenko/HERESDK-12253-PreExecutionCancel branch from d1cb9e7 to 5e49b85CompareApril 6, 2026 14:04
Tasks could be stuck on the cancellation, if task
queue is already handling IO bound tasks: usual
example is bad network connection, which forces
tasks to wait for the timeout. This prevents
tasks from being cancelled in time, and clients
forced to wait, which in turn may cause ANR in
unexpected places.
- Enable pre-execution cancellation in TaskContext by registering a
cancellation token that invokes PreExecuteCancel()
- Schedule pre-execution cancellation on the separate
cancellation lane in the task scheduler when
it is available.
Relates-To: HERESDK-12253
Signed-off-by: Mykhailo Diachenko <ext-mykhailo.z.diachenko@here.com>
@diachenko-mischa
diachenko-mischaforce-pushed the mdiachenko/HERESDK-12253-PreExecutionCancel branch from 5e49b85 to 4edc623CompareApril 7, 2026 19:19
@codecov

codecovBot commented Apr 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.59259% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.46%. Comparing base (b400ee0) to head (4edc623).
⚠️ Report is 28 commits behind head on master.

Files with missing linesPatch %Lines
...cpp-sdk-core/include/olp/core/client/TaskContext.h84.00%1 Missing and 3 partials ⚠️
...pp-sdk-core/src/thread/ThreadPoolTaskScheduler.cpp94.59%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #1689 +/- ##
==========================================
+ Coverage 80.42% 80.46% +0.04% 
==========================================
Files 351 351 Lines 14221 14280 +59 Branches 1544 1554 +10 ==========================================
+ Hits 11437 11490 +53 
Misses 2156 2156 - Partials 628 634 +6 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@khomenkd
khomenkd self-requested a review May 13, 2026 12:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@diachenko-mischa@khomenkd