Skip to content
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

Memory corruption if a fiber is resumed in shutdown handler after an exception #12529

Closed
danog opened this issue Oct 26, 2023 · 2 comments
Closed

Comments

@danog
Copy link
Contributor

danog commented Oct 26, 2023

Description

There's an issue with fibers unrelated to JIT or opcache that causes memory corruption in certain cases if a fiber is resumed in the shutdown handler after an uncaught exception triggered execution of said shutdown handler.

This memory corruption usually manifests as impossible TypeErrors when returning from random functions.

I've been seeing a lot of reports including in the amphp chat (join https://t.me/+RO_PpIt0DDZhvPPs then https://t.me/c/1156566948/47988, https://t.me/c/1156566948/47730, https://t.me/c/1156566948/47048), and got the issue myself several times, but haven't managed to reliably reproduce yet.

The only way I found of sometimes reproducing the issue is running the test.php script from https://paste.daniil.it/jit_1.tar.xz (without JIT or opcache) and randomly hitting ctrl-c multiple times during the handshake, but still it's super hard to reproduce reliably...

The issue itself is triggered by MadelineProto's shutdown handler, which resumes the revoltphp event loop (with a bit of reflection trickery) and continues execution of all fibers currently in scope, also spawning some new ones: https://github.com/danog/MadelineProto/blob/v8/src/Shutdown.php#L55

PHP Version

8.2.12

Operating System

No response

@danog
Copy link
Contributor Author

danog commented Nov 14, 2023

100% reliable reproducer: https://paste.daniil.it/ampRepro.tar.xz, repro.php script

@danog
Copy link
Contributor Author

danog commented Nov 16, 2023

Turns out this is actually an issue with revolt, fixed in revoltphp/event-loop#71 + revoltphp/event-loop#88

@danog danog closed this as completed Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants