Calls proc_open two times in a row

[mho22]

What is this PR doing?

It should correct the problem of proc_open() freezing after called one time. The freeze is due to phpwasm-emscripten-library.js line 424 await new Promise....

What problem is it solving?

Calling proc_open two times in a row will lead to a freeze due to this part of the code in the phpwasm-emscripten-library.js file :

await new Promise((resolve, reject) => {
	cp.on('spawn', resolve);
	cp.on('error', reject);
});

If spawn has already been called, it freezes indefinitely. In the test case, calling two times proc_open in a row will lead to that freeze. The solution would be to add another event listener being cp.on('exit', resolve) . If spawn has already been called, the next events should logically be stdout and exit.

Testing Instructions

node --expose-gc node_modules/nx/bin/nx run php-wasm-node:test

packages/php-wasm/node/src/test on line 500

it('Calls proc_open two times in a row', async () => {
	const result = await php.run({
		code: `<?php

                $command = "echo 'First hello world!'";

                $descriptorspec = [
                    1 => [ "pipe", "w" ],
                    2 => [ "pipe", "w" ]
                ];

                $res = proc_open( $command, $descriptorspec, $pipes );

                $stdout = stream_get_contents($pipes[1]);

                proc_close($res);

                echo $stdout;

                $command = "echo 'Second hello world!'";

                $descriptorspec = [
                    1 => [ "pipe", "w" ],
                    2 => [ "pipe", "w" ]
                ];

                $res = proc_open( $command, $descriptorspec, $pipes );

                $stdout = stream_get_contents($pipes[1]);

                proc_close($res);

                echo $stdout;`,
         });
        expect(result.text).toEqual(
            'First hello world!\nSecond hello world!\n'
        );
});

[mho22]

@adamziel, I'm not sure if adding the exit event listener in that promise is a good idea. However, what I do know is that the process freezes when it's not added. I observed that the exit and stdout events are triggered even if spawn is not called the second time. Could this indicate that the second process is using the same cp as the first one? Should there be a cp.kill() somewhere?

FYI the first commit in this PR is a failing test.

[adamziel]

Thank you for reporting and working on the fix! I investigated a bit and found that we miss out on the second spawn event due to this await here:

    cp = await PHPWASM.spawnProcess(cmdstr, argsArray);

The timeline seems to be as follows:

1. The first spawnProcess() is called
2. The await in cp = await PHPWASM. kicks in and JavaScript switches to other scheduled tasks
3. The await resumes and we have a cp value
4. The process finishes spawning and emits the spawn event
5. ... more logic happens ....
6. The second spawnProcess() is called
7. The await in cp = await PHPWASM. kicks in and JavaScript switches to other scheduled tasks
8. The process finishes spawning and emits the spawn event
9. The await resumes and we have a cp value

I'm not sure why the problem only happens on a second call – perhaps some internal value is already initialized after the first spawnProcess() the process gets spawned faster.

In any case, this seems to solve the issue:

-    cp = await PHPWASM.spawnProcess(cmdstr, argsArray);
+    cp = PHPWASM.spawnProcess(cmdstr, argsArray);
+    if (cp instanceof Promise) {
+        cp = await cp;
+    }

The solution proposed in this PR, that is waiting for exit, may still cause an infinite wait if a process awaits stdin before exiting.

[mho22]

@adamziel I am on it! There are two cp = await PHPWASM.spawnProcess(...) in phpwasm-emscripten-library.js, should I modify the two elements or only the one in js_open_process() ? I suppose I should modify each one.

[adamziel]

Both sounds good!

[mho22]

@adamziel Everything appears to be in order. 🎉 Thank you for your time.

[adamziel]

Thank you!