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

Avoid remapping freed memory in trace sender fallback #2541

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

bwoebi
Copy link
Collaborator

@bwoebi bwoebi commented Feb 27, 2024

Description

Fix faulty logic reusing a freed shm handle when sending fails.

Not sure how to test that as the precondition never should fail ... in theory.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@bwoebi bwoebi requested review from a team as code owners February 27, 2024 19:03
@bwoebi bwoebi marked this pull request as draft February 27, 2024 19:04
@codecov-commenter
Copy link

codecov-commenter commented Feb 27, 2024

Codecov Report

Merging #2541 (7c3beba) into master (380774d) will decrease coverage by 1.76%.
Report is 2 commits behind head on master.
The diff coverage is 36.18%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2541      +/-   ##
============================================
- Coverage     78.30%   76.54%   -1.76%     
  Complexity      267      267              
============================================
  Files           112      138      +26     
  Lines         13458    17459    +4001     
  Branches          0      976     +976     
============================================
+ Hits          10538    13364    +2826     
- Misses         2920     3575     +655     
- Partials          0      520     +520     
Flag Coverage Δ
appsec-extension 69.09% <ø> (?)
tracer-extension 78.69% <36.18%> (+0.46%) ⬆️
tracer-integrations 79.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
ext/integrations/integrations.c 97.88% <100.00%> (ø)
ext/priority_sampling/priority_sampling.c 95.02% <100.00%> (ø)
ext/request_hooks.c 93.24% <100.00%> (ø)
ext/handlers_curl.c 85.49% <0.00%> (ø)
ext/sidecar.h 62.50% <0.00%> (ø)
ext/startup_logging.c 89.37% <50.00%> (ø)
ext/comms_php.c 78.94% <33.33%> (ø)
ext/configuration.c 78.46% <0.00%> (ø)
ext/engine_hooks.c 85.18% <0.00%> (ø)
ext/excluded_modules.c 38.88% <0.00%> (ø)
... and 9 more

... and 34 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 380774d...7c3beba. Read the comment docs.

@pr-commenter
Copy link

pr-commenter bot commented Feb 27, 2024

Benchmarks

Benchmark execution time: 2024-02-28 18:07:42

Comparing candidate commit c61de86 in PR branch bob/fix-crash-tracesender-fallback with baseline commit 552b0ef in branch master.

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

Base automatically changed from bob/micro-optimize-shall_log to master February 28, 2024 11:21
@bwoebi bwoebi force-pushed the bob/fix-crash-tracesender-fallback branch from 24515f4 to 7c3beba Compare February 28, 2024 17:06
@bwoebi bwoebi marked this pull request as ready for review February 28, 2024 17:35
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@bwoebi bwoebi force-pushed the bob/fix-crash-tracesender-fallback branch from 7c3beba to c61de86 Compare February 28, 2024 17:36
Copy link
Contributor

@PROFeNoM PROFeNoM left a comment

Choose a reason for hiding this comment

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

No tests 😝

@bwoebi bwoebi merged commit 254782a into master Mar 1, 2024
593 of 594 checks passed
@bwoebi bwoebi deleted the bob/fix-crash-tracesender-fallback branch March 1, 2024 11:34
@github-actions github-actions bot added this to the 0.99.0 milestone Mar 1, 2024
bwoebi added a commit that referenced this pull request Mar 4, 2024
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@bwoebi bwoebi modified the milestones: 0.99.0, 0.98.1 Mar 4, 2024
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.

None yet

3 participants