Skip to content

Conversation

nielsdos
Copy link
Member

There are exit points (via bailout) in this function that do not free the recorded errors. This can cause issues when an error is emitted during shutdown as ZendMM will have shut down already but the pointer is still pointing to ZendMM memory.

This became visible after 7b3e68f

We also need to backport part of this patch to handle signal unblocking and opcache memory protection, otherwise we get issues (and errors about depth level in zend_signals).

There are exit points (via bailout) in this function that do not free the recorded errors.
This can cause issues when an error is emitted during shutdown as ZendMM will have
shut down already but the pointer is still pointing to ZendMM memory.

This became visible after 7b3e68f

We also need to backport part of this patch to handle signal unblocking and
opcache memory protection, otherwise we get issues (and errors about depth level in zend_signals).
@arnaud-lb
Copy link
Member

I think this may also happen with the opcache_compile_file(file_handle, type, &op_array); call above

@nielsdos
Copy link
Member Author

I think this may also happen with the opcache_compile_file(file_handle, type, &op_array); call above

Hmm none of the calls to opcache_compile_file surround it with a try-catch. I suppose you may be right but I need to think a bit about it.

@arnaud-lb
Copy link
Member

Ah, I likely was wrong about opcache_compile_file: it handles bailouts itself

@nielsdos
Copy link
Member Author

@arnaud-lb @iluuu1994 It appears that 16a8591 fixed this already, although I wonder if there are other bailout opportunities.

@iluuu1994
Copy link
Member

16a8591 was necessary regardless, as the code was wrong. From what I understand, the optimizer tries to avoid bailout with the zend_binary_op_produces_error() and friends functions. This is pretty fragile, so maybe there's a better, more general solution.

@arnaud-lb
Copy link
Member

The fact that cache_script_in_shared_memory() runs under HANDLE_BLOCK_INTERRUPTIONS() / SHM_UNPROTECT(), which mandates that this should be reverted in case of bailout, but was not wrapped in try-catch before, confirms that it is not supposed to bailout.

@nielsdos
Copy link
Member Author

Ack

@nielsdos nielsdos closed this Sep 30, 2025
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.

3 participants