-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add gc
and shutdown
callbacks to ZendMM custom handlers
#13432
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
Conversation
c9186cc
to
12da346
Compare
12da346
to
0366f30
Compare
fdf1e73
to
07fe9e1
Compare
07fe9e1
to
3629b00
Compare
gc
and shutdown
callbacks to ZendMM custom handlersgc
and shutdown
callbacks to ZendMM custom handlers
d476664
to
bad957f
Compare
bad957f
to
02488dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed only zend_alloc
part of the PR, and it looks almost fine.
See the comments about zend_mm_heap_create/zend_mm_heap_free
calls.
5df0ef0
to
ee2f0bc
Compare
I did an implementation in https://github.com/realFlowControl/zendmmobserver_custom_handler that shows how to use this to build an observer without relying on tricky things like tinkering with the Additionally this seems to allow to register in MINIT/MSHUTDOWN and not RINIT/RSHUTDOWN. For correct ZTS support GINIT and GSHUTDOWN should be the way to go though, I will have to play with this a bit. |
ee2f0bc
to
9d42038
Compare
9d42038
to
06ef9d5
Compare
|
@arnaud-lb in observe zendmm I played with it and with this PR, we can (un-)register ZendMM custom handlers in GINIT/GSHUTDOWN already without having any side effects from observing (forwarding back to the original heap). What is also pretty neat is that in case you want to stop observing, you can just call |
Thank you! |
Hello everyone 👋
This PR aims to add a
gc()
andshutdown()
callback to the ZendMM custom heap (currently we only havealloc()
,free()
andrealloc()
). These two new functions would allow to do garbage collection and cleanup in a custom memory manager and/or allow to handle some side effects of using thezend_mm_set_custom_handlers()
API when "just" observing the ZendMM (see #11758, "observing" as in passing calls back to ZendMM).The initial idea was to add a
zend_mm_set_custom_handlers_ex()
function to only set thegc()
andshutdown()
hooks, but this has a major drawback: It would look like you could setgc
andshutdown
handlers without callingzend_mm_set_custom_handlers()
and therefore we'd need to enforce calling the one before the other (which I'd like to refrain from doing) or it would raise complexity in handling cases where one set of handlers exists, while another does not. We could also add ause_custom_heap_ex
flag, but I'd also like to refrain from doing this, as It would add additional checks in the hot path (during (de-)allocations).I think it makes sense to have the
gc
andshutdown
bundled with the other hooks. In the current version they are nullable, but I start thinking that they should not.CC: @arnaud-lb