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

Shutdown scripts throw an exception in Windows #392

Closed
mjordan opened this issue May 31, 2017 · 8 comments
Closed

Shutdown scripts throw an exception in Windows #392

mjordan opened this issue May 31, 2017 · 8 comments
Labels

Comments

@mjordan
Copy link
Collaborator

mjordan commented May 31, 2017

Running shutdown scripts on Windows produces the following type of error:

Fatal error: Uncaught exception 'RuntimeException' with message 'Cocur\BackgroundProcess can only check if a process is running on *nix-
based systems, such as Unix, Linux or Mac OS X. You are running "WINNT".' in M:\mik_mark\vendor\cocur\background-process\src\BackgroundP
rocess.php:162
Stack trace:
#0 M:\mik_mark\vendor\cocur\background-process\src\BackgroundProcess.php(89): Cocur\BackgroundProcess\BackgroundProcess->checkSupporting
OS('Cocur\\Backgroun...')
#1 M:\mik_mark\mik(330): Cocur\BackgroundProcess\BackgroundProcess->isRunning()
#2 {main}
  thrown in M:\mik_mark\vendor\cocur\background-process\src\BackgroundProcess.php on line 162

The script ends up not running. This is an known issue with the Cocur library we are using.

This makes me wonder if we need to run shutdown scripts as background processes. It makes a lot of sense to run post-write hooks in the background so they don't slow down moving on to the next package, but that same logic doesn't apply to shutdown hooks. @jpeak5 and @MarcusBarnes, any thoughts? We can also build in logic such that if mik is running on Windows, it doesn't use Cocur, it just runs the scripts in the foreground. On Linux and OSX, it runs them in the background.

@jpeak5
Copy link
Contributor

jpeak5 commented May 31, 2017

That sounds good to me!

@mjordan
Copy link
Collaborator Author

mjordan commented Jun 1, 2017

Looking back over the background to shutdown scripts, (e.g. #156 and #354) I see no rationale for having them run in the background other than a casual mention by me in #156:

One way of implementing this would be using register_shutdown_function(), having the hook scripts run as background processes.

It may actually be misleading to to have long-running shutdown scripts run in the background since the user would not see a clear indication that it had completed.

I just ran a version of the create_structure_files.php script as a foreground job and it worked fine.

So, can we think of any reason to not run shutdown scripts only as foreground jobs, or is there any reason we should build in OS-specific logic to do this only on Windows?

@jpeak5
Copy link
Contributor

jpeak5 commented Jun 1, 2017 via email

@mjordan
Copy link
Collaborator Author

mjordan commented Jun 1, 2017

Thanks for the feedback @jpeak5. I'll wait for @MarcusBarnes to weigh in before opening a PR.

@MarcusBarnes
Copy link
Owner

MarcusBarnes commented Jun 2, 2017

A second @jpeak5's take. Thank you.

@mjordan
Copy link
Collaborator Author

mjordan commented Jun 2, 2017

OK, sounds like we have consensus. I'm happy to open a PR for evaluation.

mjordan added a commit that referenced this issue Jun 2, 2017
@mjordan mjordan mentioned this issue Jun 6, 2017
MarcusBarnes added a commit that referenced this issue Jun 8, 2017
@MarcusBarnes
Copy link
Owner

Addressed in pull-request #396 (merged with commit 9d5eae6).

@mjordan
Copy link
Collaborator Author

mjordan commented Jun 8, 2017

Thanks @MarcusBarnes, I'll update https://github.com/MarcusBarnes/mik/wiki/Shutdown-hooks to reflect this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants