Skip to content

Block sidecar notification signal while sleeping#3656

Merged
bwoebi merged 2 commits intomasterfrom
bob/dornröschenschlaf
Feb 17, 2026

Hidden character warning

The head ref may contain hidden characters: "bob/dornr\u00f6schenschlaf"
Merged

Block sidecar notification signal while sleeping#3656
bwoebi merged 2 commits intomasterfrom
bob/dornröschenschlaf

Conversation

@bwoebi
Copy link
Copy Markdown
Collaborator

@bwoebi bwoebi commented Feb 17, 2026

Otherwise users may sleep a lot less than they expect.

Otherwise users may sleep a lot less than they expect.

Signed-off-by: Bob Weinand <[email protected]>
@bwoebi bwoebi requested a review from a team as a code owner February 17, 2026 13:57
Comment thread ext/handlers_signal.c
@datadog-datadog-prod-us1
Copy link
Copy Markdown

datadog-datadog-prod-us1 Bot commented Feb 17, 2026

⚠️ Tests

Fix all issues with BitsAI or with Cursor

⚠️ Warnings

🧪 1026 Tests failed

    testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Fix with Cursor)

    testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Fix with Cursor)

testSimplePushAndProcess from laravel-8x-test.DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 69947b0400000000945b45513fb2d449
tid: 69947b0400000000
hexProcessTraceId: 945b45513fb2d449
hexProcessSpanId: 3b3249c6b336549c
processTraceId: 10690214355757356105
processSpanId: 4265552914832315548

phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:106
View all

ℹ️ Info

❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 33ee469 | Docs | Datadog PR Page | Was this helpful? Give us feedback!

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.11%. Comparing base (72a021a) to head (33ee469).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3656      +/-   ##
==========================================
- Coverage   62.21%   62.11%   -0.11%     
==========================================
  Files         141      141              
  Lines       13387    13387              
  Branches     1753     1753              
==========================================
- Hits         8329     8315      -14     
- Misses       4260     4273      +13     
- Partials      798      799       +1     

see 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72a021a...33ee469. Read the comment docs.

🚀 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.

Signed-off-by: Bob Weinand <[email protected]>
Copy link
Copy Markdown
Collaborator

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jumping through the macros, this is specifically blocking SIGVTALRM by using dd_handle_signal, yes? If I understand it right, you do not set and restore the previous set, you simply call block and unblock on SIGVTALRM. My understanding is that this should be okay but it seems a little shaky, should we not be saving and restoring?

@bwoebi
Copy link
Copy Markdown
Collaborator Author

bwoebi commented Feb 17, 2026

Why? I don't want to interfere with any possible sigmask operations happening during the calls? (except, obviously, for sigvtalrm)

@morrisonlevi
Copy link
Copy Markdown
Collaborator

Yes, I didn't think about some other wrapper also masking off a signal. Correct. Approved.

@bwoebi bwoebi merged commit 8142531 into master Feb 17, 2026
2051 of 2066 checks passed
@bwoebi bwoebi deleted the bob/dornröschenschlaf branch February 17, 2026 18:14
@github-actions github-actions Bot added this to the 1.17.0 milestone Feb 17, 2026
Sign up for free to 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.

5 participants