Skip to content

Fix unstable get_iterator pointer in shm on Windows #17034

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

iluuu1994
Copy link
Member

@cmb69 I discovered this issue when talking about something related with Arnaud. I have not verified it myself, and thus also have not verified the fix. I don't have a Windows setup, and I'm afraid it will cost me a lot of time. Would it be at all possible for you to test this? If not, no worries.

@iluuu1994 iluuu1994 force-pushed the unstable-hooked-prop-iter-ptr-on-windows branch from 074fe93 to 7cab979 Compare December 3, 2024 17:58
@cmb69
Copy link
Member

cmb69 commented Dec 3, 2024

Yeah, sure, will check this as soon as possible.

@iluuu1994 iluuu1994 force-pushed the unstable-hooked-prop-iter-ptr-on-windows branch from 7cab979 to e20914f Compare December 3, 2024 18:52
@iluuu1994
Copy link
Member Author

@cmb69 Thank you!

@iluuu1994 iluuu1994 force-pushed the unstable-hooked-prop-iter-ptr-on-windows branch from e20914f to 26a8a91 Compare December 3, 2024 22:54
@iluuu1994 iluuu1994 force-pushed the unstable-hooked-prop-iter-ptr-on-windows branch from 26a8a91 to 6c7e28a Compare December 4, 2024 00:25
@cmb69
Copy link
Member

cmb69 commented Dec 4, 2024

Thank you @iluuu1994! This looks good to me now, and I also agree that this fix is necessary for Windows; or like I would word it, for environments which allow re-attaching to OPcache SHM (currently only Windows, and always allowed there, but that might change in the future).

@iluuu1994
Copy link
Member Author

@cmb69 Thank you very much for verifying!

@iluuu1994 iluuu1994 marked this pull request as ready for review December 4, 2024 13:14
@iluuu1994 iluuu1994 requested a review from dstogov as a code owner December 4, 2024 13:14
Makes the code more self-documenting, avoiding a comment in all places. Also
makes testing easier on platforms that do not support shm reattachment.
We don't need to free inheritance_cache, since at this point we haven't gotten
around to tracking any dependencies.
@iluuu1994 iluuu1994 force-pushed the unstable-hooked-prop-iter-ptr-on-windows branch from 0d36c0e to a690e1a Compare December 9, 2024 12:43
@iluuu1994
Copy link
Member Author

@cmb69 Based on your comment, I have introduced a ZEND_OPCACHE_SHM_REATTACHMENT macro. It's not likely another platform will support shm reattachment, but at least this makes it more self-documenting.

@iluuu1994 iluuu1994 closed this in 792f63d Dec 9, 2024
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.

2 participants