-
Notifications
You must be signed in to change notification settings - Fork 377
Asyncify: Add few more function names #3037
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
base: trunk
Are you sure you want to change the base?
Asyncify: Add few more function names #3037
Conversation
|
@Utsav-Ladani thank you for contributing! @bgrgicak would you be able to review? |
|
@adamziel I tried building this for asyncify Node and it's still showing me same error. I guess more functions are missed from this list. Is there any standard way to find such function using any tool? |
|
@Utsav-Ladani Unfortunately, we don't have any standardized tool. working with these Asyncify problems is notoriously difficult. That being said, this doc page may be helpful in here: https://wordpress.github.io/wordpress-playground/developers/architecture/wasm-asyncify/#fixing-asyncify-crashes Be sure to also check the linked resources is that there's more potentially helpful content in there. |
99c2a97 to
7194c27
Compare
|
Thanks @adamziel and @bgrgicak for helpful insights. It was related to wrapping the {
"landingPage": "/wp-admin/plugins.php",
"preferredVersions": {
"php": "7.4",
"wp": "5.9"
},
"steps": [
{
"step": "login",
"username": "admin"
},
{
"step": "writeFile",
"path": "/wordpress/wp-content/plugins/crash-php-wasm.php",
"data": {
"resource": "url",
"url": "https://gist.githubusercontent.com/bgrgicak/faad52a34f909306c82800a76a936da5/raw/6e836dccb28876eec07f6dbbe588e56b8c76b0aa/php-wasm-crash.php"
}
},
{
"step": "activatePlugin",
"pluginPath": "/wordpress/wp-content/plugins/crash-php-wasm.php"
}
],
"features": {},
"login": true
}Additionally, this PR fixes a bunch of other PHP magic methods like Now the only issue is that when a method has an async call like To test that behavior, run the following code with <?php
/**
* Comprehensive PHP Magic Methods Demo
* This class demonstrates all PHP magic methods
*/
class MagicDemo
{
private $name;
public function __construct($name)
{
$this->name = $name;
}
public function __destruct()
{
echo "__destruct() called for: {$this->name}\n";
}
public function __clone()
{
$this->name = $this->name . " (cloned)";
file_get_contents("https://gist.githubusercontent.com/bgrgicak/faad52a34f909306c82800a76a936da5/raw/php-wasm-crash.php");
echo "__clone() called\n";
}
}
$obj = new MagicDemo("MyObject");
$clonedObj = clone $obj;Also, let me know the flow to recompile the PHP files. |
|
Great work @Utsav-Ladani 🚀
Could you please add this test and other test examples you mentioned to php-part-2.spec.ts? For testing, I suggest that you recompile one PHP version and run the tests using
I usually run |
|
Such a good work here @Utsav-Ladani, thank you for taking on this challenging fix! |
32605ee to
10a705b
Compare
|
I've added unit tests to I updated the Dockerfile based on the errors I encountered. Currently, with PHP Let me know how to analyze and fix such errors. |
|
@Utsav-Ladani Asyncify is very tricky in that way. If you've listed all the functions and it still throws an error, I'd poke around with setting a breakpoint, stepping over a few calls and seeing if any other function appears. Also, adding an occasional That being said, this PR already makes things a lot better and I would be super happy to get this merged even if |
6e4d212 to
275332d
Compare
Motivation for the change, related issues
Testing whether this fix the issue #2957
Implementation details
Adding few functions. Check code change for function names.
Testing Instructions (or ideally a Blueprint)
N/A