pradosoft / prado

Prado - Component Framework for PHP
Other
187 stars 70 forks source link

TProcessHelper and TSignalsDispatcher #972

Closed belisoful closed 11 months ago

belisoful commented 1 year ago

There is a problem with pcntl_fork a PRADO application: PDO connections are replicated and closing one closes all of them. In PDO, it is a feature; when the PDO object is dereferenced (aka, an object is done and no longer being used) it closes the connection. But after forking, there are two instances of the PDO representing a single connection. There is no way to exit a child thread without ending the parent PDO connection because the child PDO object is dereferenced.

The general solution is to close all connection in the parent, then reopen after the fork, as necessary, in the parent and child.

Proposal: Prado\Util\Helpers\TProcessHelper::fork that raises the global event fxPrepareForFork, then calls pcntl_fork, then raises the global event fxRestoreAfterFork.

This requires updating TDbConnection to be AutoGlobalListen = true. fxPrepareForFork will set Active = false for all db connections. It will also return [TDbConnection::objectId => bool "active status"]. The object ID is spl_object_id.

What are your thoughts on adding AutoglobalListen overhead to TDbConnections for this feature that will only be used in a rare instance?

belisoful commented 1 year ago

TDbConnection could attach and detach event handlers when Active = true and removed when Active = false. (active = false for the fxPrepareForFork does not remove the global event handlers of TDbConnection, of course)

belisoful commented 1 year ago

There is no overhead of auto global listen if the events handlers are added when active = true and removed when active = false on the TDbConnection.

Are there any other objects that need to be "corrected" for forking?

ctrlaltca commented 1 year ago

A quick solution would be to subclass TDbConnection with a single method enabling AutoglobalListen. A more comprehensive solution would be to make it somewhat configurable in application.xml, so that each class, when instanced, can detect if it needs to enable AutoGlobalListen. This could probably lead to some performance gains in applications that doesn't use fx events at all.

ctrlaltca commented 1 year ago

Are there any other objects that need to be "corrected" for forking?

Anything using php resources i guess, eg. fopen().

belisoful commented 1 year ago

That's a great suggestion. the new IO package encapsulates the fopen, socket_create, popen, and proc_open. They do not exactly have the same issues with auto-destructing on dereference. They can be dereferenced without closing.

However, tmpfile() does actually do the same thing. there is a TTempStream class for encapsulating that function. It is a derivative of fopen. tmpfile does actually do the auto remove on dereference.

hmmm. Thank you for your comment on this. It makes me think that we should use a PRADO managed tmp file as default rather than tmpfile() as a default due to tmpfile not being thread safe, as per your indicator.

belisoful commented 1 year ago

Let me check that fopen Dereferenced resource does orphan the resource in the child...

belisoful commented 1 year ago

ChatGPT says (and verifies itself) that a Child process exiting without fclose a file/stream resource will preserve the resource in the parent. The the tmpfile() will close its resource automatically on child exit because it works like PDO to auto self destruct.

belisoful commented 1 year ago

I like the idea of a behavior adding these event handlers (of the owner). This would be the performant way. TDataSourceConfig module having module behaviors (passed to the TDbConnection) that would attach the event handlers. I likeTForkableBehavior as a name for adding these event handlers to the suggested global events.

And the TTempStream, when using tmpfile(), the class needs to copy the tmpfile() on fxPrepareForFork and pass it to fxRestoreAfterFork. The parent then copies its own tmpfile to a new tmpfile, and the child copies its tmpfile to its new tmpfile(). When the original tmpfile and original tmpfile copy (for the child) are dereferenced, they are unlink and then both the parent and child maintain their new tmpfile.

The automatic file deletion can be turned off for Prado managed tmp files and not need to do all that copying of tmp files.

belisoful commented 1 year ago

I found this interesting code and comment at https://www.php.net/manual/en/function.pcntl-fork.php#127790

// Explicity kill process, or else the resource shared with parent will be closed
posix_kill(getmypid(), SIGTERM);
exit();

This may be the solution to PDO and tmpfile auto-detonating on dereferencing in children.

This may render the global events for this unnecessary.... testing.

belisoful commented 1 year ago

The way I'm thinking of implementing this, if it works, as an extension of the TExitException.

belisoful commented 1 year ago

If the TChildExitException with a Terminate Signal is used, the app resources shouldn't be replicated. and visa versa. I'm thinking that an app level behavior is needed. The TForkable[Behavior] on an application would add the TForkable to TTDbConnection class behaviors, which adds its fxPrepareForFork and fxRestoreAfterFork events to the global event handler.

This is getting interesting with behaviors.

belisoful commented 1 year ago

Here is my latest thinking: 1) if TDbConnection goes Active = false, on fxPrepareForFork and Active = true on fxRestoreAfterFork, then calling SIG_TERM on exit for the child will leave the newly open child DB hanging. so a third global event for closing up child processes to clean up any resources that need to be closed before SIG_TERM is sent and process exit. 2) if TDBConnection is not closed and reopened on forking, then SIG_TERM is acceptable.

Here is the implementation::

belisoful commented 1 year ago

After lots of research, the new PHP8 doesn't have resource issues in forking. I tried making a child corrupt the parent PDO Sqlite3 connection and a tmpfile resource but failed to recreate the issues I was having in prior versions of PHP. It was easy to fork a cron thread that exited and corrupted the parent PDO DB Connection. Not so much now.

Maybe PHP is incrementing resource reference counters so they do not deference on exiting the child. Whatever it is, these solutions solve no problem.

Closing issue.

belisoful commented 1 year ago

After much thinking, I think there is a use case for these methods... Capturing the Application log of children and injecting it back into the main thread.

An application behavior that connects the two threads would transfer the log from the child to the parent at the end of execution.