Skip to content

Conversation

@bwoebi
Copy link
Collaborator

@bwoebi bwoebi commented Jan 20, 2026

Once any ZTS thread shuts down, _shared_state is munmap'ped(). It will be unmapped and any other later mapping may happen at that same place through bad luck. If another thread shuts down, it will munmap this again and accesses to the later mapping are now crashing.

Fixes #3511.

@bwoebi bwoebi requested a review from a team as a code owner January 20, 2026 18:06
Once any ZTS thread shuts down, _shared_state is munmap'ped().
It will be unmapped and any other later mapping may happen at that same place through bad luck. If another thread shuts down, it will munmap this again and accesses to the later mapping are now crashing.

Signed-off-by: Bob Weinand <[email protected]>
@bwoebi bwoebi force-pushed the bob/fix-appsec-munmap branch from 1dac74b to 367c476 Compare January 20, 2026 18:07
@bwoebi bwoebi requested a review from a team as a code owner January 20, 2026 18:07
Copy link
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.

In essence, the previous code was doing a per-thread shutdown, but this mapping is global, not thread-scoped. The fix is clear and makes sense, unlike the symptoms observed while debugging this thing. If this fix is not totally right, it seems clearly in the right direction.

@datadog-official
Copy link

datadog-official bot commented Jan 20, 2026

⚠️ Tests

Fix all issues with Cursor

⚠️ Warnings

🧪 1024 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: 696fc654000000008391c8359d5ffe5a
tid: 696fc65400000000
hexProcessTraceId: 8391c8359d5ffe5a
hexProcessSpanId: 2155c0da55202a47
processTraceId: 9480578823190740570
processSpanId: 2402038020235602503
View all

ℹ️ Info

❄️ No new flaky tests detected

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

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.91%. Comparing base (7d6fda9) to head (367c476).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
appsec/src/extension/helper_process.c 80.00% 0 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (80.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3590      +/-   ##
==========================================
- Coverage   62.00%   61.91%   -0.09%     
==========================================
  Files         140      140              
  Lines       13311    13312       +1     
  Branches     1762     1762              
==========================================
- Hits         8253     8242      -11     
- Misses       4270     4280      +10     
- Partials      788      790       +2     
Files with missing lines Coverage Δ
appsec/src/extension/helper_process.c 52.90% <80.00%> (+0.15%) ⬆️

... and 2 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 7d6fda9...367c476. 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.

@pr-commenter
Copy link

pr-commenter bot commented Jan 20, 2026

Benchmarks [ appsec ]

Benchmark execution time: 2026-01-20 18:47:07

Comparing candidate commit 367c476 in PR branch bob/fix-appsec-munmap with baseline commit 7d6fda9 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.

@bwoebi bwoebi merged commit efd9fb9 into master Jan 21, 2026
2006 of 2009 checks passed
@bwoebi bwoebi deleted the bob/fix-appsec-munmap branch January 21, 2026 11:44
@github-actions github-actions bot added this to the 1.17.0 milestone Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: SegFault with parallel extension in v1.13.1

5 participants