Skip to content

Fix double end hook run/segault when blocking in PHP 7.x#3490

Merged
cataphract merged 1 commit intomasterfrom
glopes/hook-end-segfault-php7
Dec 5, 2025
Merged

Fix double end hook run/segault when blocking in PHP 7.x#3490
cataphract merged 1 commit intomasterfrom
glopes/hook-end-segfault-php7

Conversation

@cataphract
Copy link
Copy Markdown
Contributor

Description

End hooks run twice if appsec blocks during one in PHP 7, possibly segfaulting.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@cataphract cataphract requested a review from a team as a code owner November 18, 2025 12:20
@cataphract cataphract force-pushed the glopes/hook-end-segfault-php7 branch from ebbc709 to 3421bbf Compare November 18, 2025 12:22
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Nov 18, 2025

Benchmarks [ tracer ]

Benchmark execution time: 2025-12-05 13:42:34

Comparing candidate commit 50eab8d in PR branch glopes/hook-end-segfault-php7 with baseline commit ca291a7 in branch master.

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

scenario:ComposerTelemetryBench/benchTelemetryParsing-opcache

  • 🟩 execution_time [-1.653µs; -0.747µs] or [-12.152%; -5.495%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟥 execution_time [+3.652µs; +5.148µs] or [+3.598%; +5.071%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟥 execution_time [+6.919µs; +8.381µs] or [+6.681%; +8.092%]

scenario:PDOBench/benchPDOOverhead

  • 🟥 execution_time [+4.620µs; +8.682µs] or [+2.065%; +3.880%]

scenario:PDOBench/benchPDOOverheadWithDBM

  • 🟥 execution_time [+5.951µs; +9.046µs] or [+2.675%; +4.066%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟩 execution_time [-43.791µs; -30.109µs] or [-9.714%; -6.679%]

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.72%. Comparing base (ca291a7) to head (50eab8d).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3490      +/-   ##
==========================================
+ Coverage   61.65%   61.72%   +0.07%     
==========================================
  Files         143      143              
  Lines       13004    13004              
  Branches     1702     1702              
==========================================
+ Hits         8017     8027      +10     
+ Misses       4225     4218       -7     
+ Partials      762      759       -3     

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 ca291a7...50eab8d. 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.

cataphract added a commit that referenced this pull request Nov 20, 2025
cataphract added a commit that referenced this pull request Nov 21, 2025
@cataphract cataphract force-pushed the glopes/hook-end-segfault-php7 branch from 3421bbf to 3af54b5 Compare December 5, 2025 12:34
@cataphract cataphract force-pushed the glopes/hook-end-segfault-php7 branch from 3af54b5 to 50eab8d Compare December 5, 2025 12:35
@cataphract cataphract merged commit 1dff083 into master Dec 5, 2025
2006 of 2007 checks passed
@cataphract cataphract deleted the glopes/hook-end-segfault-php7 branch December 5, 2025 15:24
@github-actions github-actions Bot added this to the 1.15.0 milestone Dec 5, 2025
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.

3 participants