Skip to content

feat(ext): add exception to data in end callback #2

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.

Sign up for GitHub

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

Open
Litarnus wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(ext): add exception to data in end callback #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa7831e
feat(ext): add exception to data in end callback
Litarnus Jun 16, 2026
2ea9263
add test
Litarnus Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions sentry.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,15 @@ static void sentry_observer_end(zend_execute_data *execute_data, zval *return_va
ZVAL_COPY(&metadata_zv, &state->metadata);
add_assoc_zval(&event, "metadata", &metadata_zv);

zend_object *exception = EG(exception);
if (exception != NULL) {
zval exception_zv;
ZVAL_OBJ_COPY(&exception_zv, exception);
add_assoc_zval(&event, "exception", &exception_zv);
} else {
add_assoc_null(&event, "exception");
}
Comment on lines +515 to +522

@sentry sentry Bot Jun 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: A reference to the original exception is decremented twice in the unwind-exit path (e.g., from die()), causing a premature free and potential use-after-free.
Severity: HIGH

Suggested Fix

The zval_ptr_dtor(&event) call in sentry_observer_end should be skipped or handled conditionally when an unwind-exit occurs. The reference to the original exception is already released within sentry_exception_isolation_end in this specific path, so the second decrement causes the object to be freed too early.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry.c#L515-L522

Potential issue: In the specific case where an instrumented function triggers an
unwind-exit by calling `exit()` or `die()`, a reference counting issue leads to a
premature free of the original exception object. The code adds the exception to an
`event` array via `ZVAL_OBJ_COPY`, incrementing its reference count. The unwind-exit
path correctly releases its own reference via `OBJ_RELEASE`. However,
`sentry_observer_end` later calls `zval_ptr_dtor(&event)`, which decrements the count
again, freeing the object. The observer framework may still require this object during
stack unwinding, leading to a use-after-free.

Did we get this right? 👍 / 👎 to inform future reviews.

@Litarnus Litarnus Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not sure this is correct. ZVAL_OBJ_COPY increases the ref-count and zval_ptr_dtor(&event) will release each element, meaning that it will just decrement the ref-count.


ZVAL_COPY_VALUE(&params[0], &event);

ZVAL_COPY_VALUE(&params[1], &state->user_state);
Expand Down
45 changes: 45 additions & 0 deletions tests/test_cought_exceptions_do_not_leak.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
--TEST--
Tests that re-throwing different exceptions properly populates the 'exception' key.
--EXTENSIONS--
sentry
--FILE--
<?php

class CustomException extends Exception {}

function test_throw() {
throw new CustomException("Oh no");
}

function test_rethrow() {
try {
test_throw();
} catch (Throwable $t) {
echo "Exception caught" . PHP_EOL;
}
}

\Sentry\setEndCallback(static function (array $data) {
$exception = $data['exception'];
if ($exception !== null) {
echo $exception->getMessage() . PHP_EOL;
echo get_class($exception) . PHP_EOL;
} else {
echo 'No exception here' . PHP_EOL;
}
});

\Sentry\instrument(null, 'test_throw');
\Sentry\instrument(null, 'test_rethrow');
try {
test_rethrow();
} catch (Throwable $t) {

}

?>
--EXPECTF--
Oh no
CustomException
Exception caught
No exception here
32 changes: 32 additions & 0 deletions tests/test_exception_set_in_end_callback.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
Tests that a thrown exception is captured and stored under the 'exception' key in the end callback
--EXTENSIONS--
sentry
--FILE--
<?php

class CustomException extends Exception {

}

function test_throw() {
throw new CustomException("Oh no");
}

\Sentry\setEndCallback(static function (array $data) {
$exception = $data['exception'];
echo $exception->getMessage() . PHP_EOL;
echo get_class($exception) . PHP_EOL;
});

\Sentry\instrument(null, 'test_throw');
try {
test_throw();
} catch (Throwable $t) {

}

?>
--EXPECTF--
Oh no
CustomException
40 changes: 40 additions & 0 deletions tests/test_fields_always_present.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
--TEST--
Tests that some fields are always present in the start and end callback and also assert their type.
--EXTENSIONS--
sentry
--FILE--
<?php

function test_instrumented() {
return 10;
}

\Sentry\setStartCallback(static function (array $data) {
echo 'Start callback' . PHP_EOL;
echo 'start_time: ' . get_debug_type($data['start_time']) . PHP_EOL;
echo 'name: ' . get_debug_type($data['name']) . PHP_EOL;
});

\Sentry\setEndCallback(static function (array $data) {
echo 'End callback' . PHP_EOL;
echo 'start_time: ' . get_debug_type($data['start_time']) . PHP_EOL;
echo 'name: ' . get_debug_type($data['name']) . PHP_EOL;
echo 'duration: ' . get_debug_type($data['duration']) . PHP_EOL;
echo 'end_time: ' . get_debug_type($data['end_time']) . PHP_EOL;
echo 'metadata: ' . get_debug_type($data['metadata']) . PHP_EOL;
});

\Sentry\instrument(null, 'test_instrumented');
test_instrumented();

?>
--EXPECTF--
Start callback
start_time: float
name: string
End callback
start_time: float
name: string
duration: float
end_time: float
metadata: array
27 changes: 27 additions & 0 deletions tests/test_metadata_key_always_exists.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
Tests that the 'metadata' key in data in the end callback is always populated and never null (Function)
--EXTENSIONS--
sentry
--FILE--
<?php

function test_instrumented() {
return 10;
}

\Sentry\setEndCallback(static function (array $data) {
if (!isset($data['metadata'])) {
echo "metadata array is not set" . PHP_EOL;
} else if ($data['metadata'] === null) {
echo "metadata array is null" . PHP_EOL;
} else {
echo "[]" . PHP_EOL;
}
});

\Sentry\instrument(null, 'test_instrumented');
test_instrumented();

?>
--EXPECTF--
[]
27 changes: 27 additions & 0 deletions tests/test_metadata_key_always_exists_attribute.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
Tests that the 'metadata' key in data in the end callback is always populated and never null (Attribute)
--EXTENSIONS--
sentry
--FILE--
<?php

#[\Sentry\Trace]
function test_instrumented() {
return 10;
}

\Sentry\setEndCallback(static function (array $data) {
if (!isset($data['metadata'])) {
echo "metadata array is not set" . PHP_EOL;
} else if ($data['metadata'] === null) {
echo "metadata array is null" . PHP_EOL;
} else {
echo "[]" . PHP_EOL;
}
});

test_instrumented();

?>
--EXPECTF--
[]
43 changes: 43 additions & 0 deletions tests/test_nested_exceptions_properly_scoped.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
Tests that catching an exception in a function will make 'exception' be null in the end callback
--EXTENSIONS--
sentry
--FILE--
<?php

class CustomException extends Exception {}

class TestException extends Exception {}

function test_throw() {
throw new CustomException("Oh no");
}

function test_rethrow() {
try {
test_throw();
} catch (Throwable $t) {
throw new TestException("throw different exception");
}
}

\Sentry\setEndCallback(static function (array $data) {
$exception = $data['exception'];
echo $exception->getMessage() . PHP_EOL;
echo get_class($exception) . PHP_EOL;
});

\Sentry\instrument(null, 'test_throw');
\Sentry\instrument(null, 'test_rethrow');
try {
test_rethrow();
} catch (Throwable $t) {

}

?>
--EXPECTF--
Oh no
CustomException
throw different exception
TestException
Loading