diff --git a/config/nativephp.php b/config/nativephp.php index 08ed4177..50df403b 100644 --- a/config/nativephp.php +++ b/config/nativephp.php @@ -1,5 +1,7 @@ \App\Providers\NativeAppServiceProvider::class, + 'provider' => NativeAppServiceProvider::class, /** * A list of environment keys that should be removed from the diff --git a/resources/electron/electron-plugin/dist/index.js b/resources/electron/electron-plugin/dist/index.js index 959976e1..94ef945c 100644 --- a/resources/electron/electron-plugin/dist/index.js +++ b/resources/electron/electron-plugin/dist/index.js @@ -24,6 +24,7 @@ class NativePHP { this.processes = []; this.mainWindow = null; this.schedulerInterval = undefined; + this.quitting = false; } bootstrap(app, icon, phpBinary, cert, appPath) { initialize(); @@ -52,13 +53,20 @@ class NativePHP { app.quit(); } }); - app.on('before-quit', () => { - if (this.schedulerInterval) { - clearInterval(this.schedulerInterval); + app.on('before-quit', (event) => __awaiter(this, void 0, void 0, function* () { + if (this.quitting) { + return; } - stopAllProcesses(); + this.quitting = true; + event.preventDefault(); this.killChildProcesses(); - }); + stopAllProcesses(); + const deadline = Date.now() + 12000; + while (Object.keys(state.processes).length > 0 && Date.now() < deadline) { + yield new Promise((resolve) => setTimeout(resolve, 200)); + } + app.quit(); + })); app.on('browser-window-created', (_, window) => { optimizer.watchWindowShortcuts(window); }); diff --git a/resources/electron/electron-plugin/dist/server/api/childProcess.js b/resources/electron/electron-plugin/dist/server/api/childProcess.js index 0d3eee2e..fbea55c5 100644 --- a/resources/electron/electron-plugin/dist/server/api/childProcess.js +++ b/resources/electron/electron-plugin/dist/server/api/childProcess.js @@ -137,11 +137,17 @@ function stopProcess(alias) { if (proc === undefined) { return; } - state.processes[alias].settings.persistent = false; + const settings = state.processes[alias].settings; + settings.persistent = false; console.log('Process [' + alias + '] stopping with PID [' + proc.pid + '].'); try { - killSync(proc.pid, 'SIGTERM', true); - proc.kill(); + if (settings.handlesOwnShutdown && process.platform !== 'win32') { + process.kill(proc.pid, 'SIGTERM'); + } + else { + killSync(proc.pid, 'SIGTERM', true); + proc.kill(); + } } catch (_a) { console.log('Process [' + alias + '] already exited — nothing to kill.'); diff --git a/resources/electron/electron-plugin/src/index.ts b/resources/electron/electron-plugin/src/index.ts index 592e3bfc..7cc1f7cd 100644 --- a/resources/electron/electron-plugin/src/index.ts +++ b/resources/electron/electron-plugin/src/index.ts @@ -25,6 +25,7 @@ class NativePHP { processes: ChildProcessWithoutNullStreams[] = []; mainWindow = null; schedulerInterval = undefined; + quitting = false; public bootstrap(app: CrossProcessExports.App, icon: string, phpBinary: string, cert: string, appPath: string) { initialize(); @@ -59,15 +60,34 @@ class NativePHP { } }); - app.on('before-quit', () => { - if (this.schedulerInterval) { - clearInterval(this.schedulerInterval); + app.on('before-quit', async (event) => { + // We call app.quit() again at the end, which fires this handler a + // second time. Let that pass straight through so the quit happens. + if (this.quitting) { + return; } + this.quitting = true; + event.preventDefault(); + + // Stop the framework's own processes first (the PHP server and the + // like). While they're still up, an incoming request could boot the + // app and spawn fresh child processes that we'd never clean up here. + this.killChildProcesses(); - // close all child processes from the app + // Now the app's child processes. The ones started with `handlesOwnShutdown` + // get a plain SIGTERM rather than a tree-kill, so they can bring down + // their own children themselves before they exit. stopAllProcesses(); - this.killChildProcesses(); + // Give them a moment to act on that SIGTERM (flush, persist, whatever + // they need) before we pull the plug. Each one drops out of state as + // it exits; the deadline stops a stuck process from blocking the quit. + const deadline = Date.now() + 12_000; + while (Object.keys(state.processes).length > 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 200)); + } + + app.quit(); }); // Default open or close DevTools by F12 in development diff --git a/resources/electron/electron-plugin/src/server/api/childProcess.ts b/resources/electron/electron-plugin/src/server/api/childProcess.ts index df2393fa..36c74e6d 100644 --- a/resources/electron/electron-plugin/src/server/api/childProcess.ts +++ b/resources/electron/electron-plugin/src/server/api/childProcess.ts @@ -194,14 +194,25 @@ function stopProcess(alias) { } // Set persistent to false and prevent the process from restarting. - state.processes[alias].settings.persistent = false; + const settings = state.processes[alias].settings; + settings.persistent = false; console.log('Process [' + alias + '] stopping with PID [' + proc.pid + '].'); try { - // @ts-ignore - killSync(proc.pid, 'SIGTERM', true); // Kill tree - proc.kill(); // Does not work but just in case. (do not put before killSync) + if (settings.handlesOwnShutdown && process.platform !== 'win32') { + // Signal this process alone, leaving its children untouched. A + // process that manages its own long-running children can then shut + // them down on its own terms before it exits, rather than having + // them killed underneath it by a tree-kill. Windows has no real + // signals (a kill there is always immediate), so we fall back to + // the tree-kill below instead of leaving the children orphaned. + process.kill(proc.pid, 'SIGTERM'); + } else { + // @ts-ignore + killSync(proc.pid, 'SIGTERM', true); // Kill tree + proc.kill(); // Does not work but just in case. (do not put before killSync) + } } catch { console.log('Process [' + alias + '] already exited — nothing to kill.'); } diff --git a/src/ChildProcess.php b/src/ChildProcess.php index 9c691b80..a3b8024e 100644 --- a/src/ChildProcess.php +++ b/src/ChildProcess.php @@ -22,6 +22,8 @@ class ChildProcess implements ChildProcessContract public readonly bool $persistent; + public readonly bool $handlesOwnShutdown; + public readonly ?array $iniSettings; final public function __construct(protected Client $client) {} @@ -66,7 +68,8 @@ public function start( string $alias, ?string $cwd = null, ?array $env = null, - bool $persistent = false + bool $persistent = false, + bool $handlesOwnShutdown = false ): self { $cmd = $this->parseCommand($cmd); @@ -76,6 +79,7 @@ public function start( 'cwd' => $cwd ?? base_path(), 'env' => $env, 'persistent' => $persistent, + 'handlesOwnShutdown' => $handlesOwnShutdown, ])->json(); return $this->fromRuntimeProcess($process); @@ -85,7 +89,7 @@ public function start( * @param string|string[] $cmd * @return $this */ - public function php(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null): self + public function php(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null, bool $handlesOwnShutdown = false): self { $cmd = $this->parseCommand($cmd); @@ -96,12 +100,13 @@ public function php(string|array $cmd, string $alias, ?array $env = null, ?bool 'env' => $env, 'persistent' => $persistent, 'iniSettings' => $iniSettings, + 'handlesOwnShutdown' => $handlesOwnShutdown, ])->json(); return $this->fromRuntimeProcess($process); } - public function node(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false): self + public function node(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, bool $handlesOwnShutdown = false): self { $cmd = $this->parseCommand($cmd); @@ -111,6 +116,7 @@ public function node(string|array $cmd, string $alias, ?array $env = null, ?bool 'cwd' => base_path(), 'env' => $env, 'persistent' => $persistent, + 'handlesOwnShutdown' => $handlesOwnShutdown, ])->json(); return $this->fromRuntimeProcess($process); @@ -120,13 +126,13 @@ public function node(string|array $cmd, string $alias, ?array $env = null, ?bool * @param string|string[] $cmd * @return $this */ - public function artisan(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null): self + public function artisan(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null, bool $handlesOwnShutdown = false): self { $cmd = $this->parseCommand($cmd); $cmd = ['artisan', ...$cmd]; - return $this->php($cmd, $alias, env: $env, persistent: $persistent, iniSettings: $iniSettings); + return $this->php($cmd, $alias, env: $env, persistent: $persistent, iniSettings: $iniSettings, handlesOwnShutdown: $handlesOwnShutdown); } public function stop(?string $alias = null): void diff --git a/src/Contracts/ChildProcess.php b/src/Contracts/ChildProcess.php index c11313dd..0f007b1a 100644 --- a/src/Contracts/ChildProcess.php +++ b/src/Contracts/ChildProcess.php @@ -13,14 +13,15 @@ public function start( string $alias, ?string $cwd = null, ?array $env = null, - bool $persistent = false + bool $persistent = false, + bool $handlesOwnShutdown = false ): self; - public function php(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null): self; + public function php(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null, bool $handlesOwnShutdown = false): self; - public function artisan(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null): self; + public function artisan(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, ?array $iniSettings = null, bool $handlesOwnShutdown = false): self; - public function node(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false): self; + public function node(string|array $cmd, string $alias, ?array $env = null, ?bool $persistent = false, bool $handlesOwnShutdown = false): self; public function stop(?string $alias = null): void; diff --git a/src/Drivers/Electron/Updater/UpdaterManager.php b/src/Drivers/Electron/Updater/UpdaterManager.php index 5ee904db..2df11012 100644 --- a/src/Drivers/Electron/Updater/UpdaterManager.php +++ b/src/Drivers/Electron/Updater/UpdaterManager.php @@ -2,14 +2,16 @@ namespace Native\Desktop\Drivers\Electron\Updater; +use Illuminate\Contracts\Foundation\Application; use InvalidArgumentException; +use Native\Desktop\Drivers\Electron\Updater\Contracts\Updater; class UpdaterManager { /** * The application instance. * - * @var \Illuminate\Contracts\Foundation\Application + * @var Application */ protected $app; @@ -23,7 +25,7 @@ class UpdaterManager /** * Create a new Updater manager instance. * - * @param \Illuminate\Contracts\Foundation\Application $app + * @param Application $app * @return void */ public function __construct($app) @@ -35,7 +37,7 @@ public function __construct($app) * Get a updater provider instance by name, wrapped in a repository. * * @param string|null $name - * @return \Native\Desktop\Drivers\Electron\Updater\Contracts\Updater + * @return Updater */ public function provider($name = null) { @@ -48,7 +50,7 @@ public function provider($name = null) * Get a updater provider instance. * * @param string|null $driver - * @return \Native\Desktop\Drivers\Electron\Updater\Contracts\Updater + * @return Updater */ public function driver($driver = null) { @@ -59,9 +61,9 @@ public function driver($driver = null) * Resolve the given store. * * @param string $name - * @return \Native\Desktop\Drivers\Electron\Updater\Contracts\Updater + * @return Updater * - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function resolve($name) { @@ -119,7 +121,7 @@ public function setDefaultDriver($name) /** * Set the application instance used by the manager. * - * @param \Illuminate\Contracts\Foundation\Application $app + * @param Application $app * @return $this */ public function setApplication($app) @@ -132,7 +134,7 @@ public function setApplication($app) /** * Create an instance of the spaces updater driver. * - * @return \Native\Desktop\Drivers\Electron\Updater\Contracts\Updater + * @return Updater */ protected function createSpacesDriver(array $config) { @@ -142,7 +144,7 @@ protected function createSpacesDriver(array $config) /** * Create an instance of the spaces updater driver. * - * @return \Native\Desktop\Drivers\Electron\Updater\Contracts\Updater + * @return Updater */ protected function createS3Driver(array $config) { @@ -152,7 +154,7 @@ protected function createS3Driver(array $config) /** * Create an instance of the GitHub updater driver. * - * @return \Native\Desktop\Drivers\Electron\Updater\Contracts\Updater + * @return Updater */ protected function createGitHubDriver(array $config) { diff --git a/src/Facades/ChildProcess.php b/src/Facades/ChildProcess.php index 6a12bebf..b335176b 100644 --- a/src/Facades/ChildProcess.php +++ b/src/Facades/ChildProcess.php @@ -11,10 +11,10 @@ * @method static \Native\Desktop\ChildProcess|null get(string $alias = null) * @method static \Native\Desktop\ChildProcess message(string $message, string $alias = null) * @method static \Native\Desktop\ChildProcess restart(string $alias = null) - * @method static \Native\Desktop\ChildProcess start(string|array $cmd, string $alias, string $cwd = null, array $env = null, bool $persistent = false) - * @method static \Native\Desktop\ChildProcess node(string|array $cmd, string $alias, array $env = null, bool $persistent = false) - * @method static \Native\Desktop\ChildProcess php(string|array $cmd, string $alias, array $env = null, bool $persistent = false, ?array $iniSettings = null) - * @method static \Native\Desktop\ChildProcess artisan(string|array $cmd, string $alias, array $env = null, bool $persistent = false, ?array $iniSettings = null) + * @method static \Native\Desktop\ChildProcess start(string|array $cmd, string $alias, string $cwd = null, array $env = null, bool $persistent = false, bool $handlesOwnShutdown = false) + * @method static \Native\Desktop\ChildProcess node(string|array $cmd, string $alias, array $env = null, bool $persistent = false, bool $handlesOwnShutdown = false) + * @method static \Native\Desktop\ChildProcess php(string|array $cmd, string $alias, array $env = null, bool $persistent = false, ?array $iniSettings = null, bool $handlesOwnShutdown = false) + * @method static \Native\Desktop\ChildProcess artisan(string|array $cmd, string $alias, array $env = null, bool $persistent = false, ?array $iniSettings = null, bool $handlesOwnShutdown = false) * @method static void stop(string $alias = null) * @method static static when($value = null, ?callable $callback = null, ?callable $default = null) * @method static static unless($value = null, ?callable $callback = null, ?callable $default = null) diff --git a/src/Facades/Menu.php b/src/Facades/Menu.php index 53e162e1..c93a5493 100644 --- a/src/Facades/Menu.php +++ b/src/Facades/Menu.php @@ -10,6 +10,7 @@ use Native\Desktop\Menu\Items\Radio; use Native\Desktop\Menu\Items\Role; use Native\Desktop\Menu\Items\Separator; +use Native\Desktop\Menu\MenuBuilder; /** * @method static \Native\Desktop\Menu\Menu make(MenuItem ...$items) @@ -48,6 +49,6 @@ class Menu extends Facade { protected static function getFacadeAccessor() { - return \Native\Desktop\Menu\MenuBuilder::class; + return MenuBuilder::class; } } diff --git a/src/Facades/MenuBar.php b/src/Facades/MenuBar.php index 162f16e2..c067e890 100644 --- a/src/Facades/MenuBar.php +++ b/src/Facades/MenuBar.php @@ -4,6 +4,7 @@ use Illuminate\Support\Facades\Facade; use Native\Desktop\Menu\Menu; +use Native\Desktop\MenuBar\MenuBarManager; /** * @method static \Native\Desktop\MenuBar\PendingCreateMenuBar create() @@ -21,6 +22,6 @@ class MenuBar extends Facade { protected static function getFacadeAccessor() { - return \Native\Desktop\MenuBar\MenuBarManager::class; + return MenuBarManager::class; } } diff --git a/src/Fakes/ChildProcessFake.php b/src/Fakes/ChildProcessFake.php index b850eb7b..93b9db76 100644 --- a/src/Fakes/ChildProcessFake.php +++ b/src/Fakes/ChildProcessFake.php @@ -68,15 +68,16 @@ public function start( string $alias, ?string $cwd = null, ?array $env = null, - bool $persistent = false + bool $persistent = false, + ?bool $handlesOwnShutdown = null ): self { - $this->starts[] = [ + $this->starts[] = $this->withHandlesOwnShutdown([ 'cmd' => $cmd, 'alias' => $alias, 'cwd' => $cwd, 'env' => $env, 'persistent' => $persistent, - ]; + ], $handlesOwnShutdown); return $this; } @@ -86,15 +87,16 @@ public function php( string $alias, ?array $env = null, ?bool $persistent = false, - ?array $iniSettings = null + ?array $iniSettings = null, + ?bool $handlesOwnShutdown = null ): self { - $this->phps[] = [ + $this->phps[] = $this->withHandlesOwnShutdown([ 'cmd' => $cmd, 'alias' => $alias, 'env' => $env, 'persistent' => $persistent, 'iniSettings' => $iniSettings, - ]; + ], $handlesOwnShutdown); return $this; } @@ -104,15 +106,16 @@ public function artisan( string $alias, ?array $env = null, ?bool $persistent = false, - ?array $iniSettings = null + ?array $iniSettings = null, + ?bool $handlesOwnShutdown = null ): self { - $this->artisans[] = [ + $this->artisans[] = $this->withHandlesOwnShutdown([ 'cmd' => $cmd, 'alias' => $alias, 'env' => $env, 'persistent' => $persistent, 'iniSettings' => $iniSettings, - ]; + ], $handlesOwnShutdown); return $this; } @@ -121,18 +124,43 @@ public function node( string|array $cmd, string $alias, ?array $env = null, - ?bool $persistent = false + ?bool $persistent = false, + ?bool $handlesOwnShutdown = null ): self { - $this->nodes[] = [ + $this->nodes[] = $this->withHandlesOwnShutdown([ 'cmd' => $cmd, 'alias' => $alias, 'env' => $env, 'persistent' => $persistent, - ]; + ], $handlesOwnShutdown); return $this; } + /** + * Add the handlesOwnShutdown flag to a recorded call, but only when it was + * actually passed. The assert*() helpers spread each recorded call into + * the callback as named arguments, so always including the key would hand + * a `handlesOwnShutdown:` argument to every existing callback and break the ones + * that don't accept it. Leaving it out when unset keeps those working, + * while callers who set it can still assert against it. + * + * This only exists for backwards compatibility. Once we're free to break + * that in a major release, drop the helper and record handlesOwnShutdown inline + * alongside the other keys. + * + * @param array $call + * @return array + */ + private function withHandlesOwnShutdown(array $call, ?bool $handlesOwnShutdown): array + { + if ($handlesOwnShutdown !== null) { + $call['handlesOwnShutdown'] = $handlesOwnShutdown; + } + + return $call; + } + public function stop(?string $alias = null): void { $this->stops[] = $alias; diff --git a/src/Http/Middleware/OptionalNightwatchNever.php b/src/Http/Middleware/OptionalNightwatchNever.php index 9bd9df06..7cf3bd2d 100644 --- a/src/Http/Middleware/OptionalNightwatchNever.php +++ b/src/Http/Middleware/OptionalNightwatchNever.php @@ -4,13 +4,14 @@ use Closure; use Illuminate\Http\Request; +use Laravel\Nightwatch\Http\Middleware\Sample; class OptionalNightwatchNever { public function handle(Request $request, Closure $next): mixed { - if (class_exists(\Laravel\Nightwatch\Http\Middleware\Sample::class)) { - $middleware = app(\Laravel\Nightwatch\Http\Middleware\Sample::class); + if (class_exists(Sample::class)) { + $middleware = app(Sample::class); return $middleware->handle($request, $next, 0.0); } diff --git a/src/NativeServiceProvider.php b/src/NativeServiceProvider.php index 4d240ddd..138a7510 100644 --- a/src/NativeServiceProvider.php +++ b/src/NativeServiceProvider.php @@ -3,6 +3,7 @@ namespace Native\Desktop; use Illuminate\Console\Application; +use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Foundation\Application as Foundation; use Illuminate\Foundation\Http\Kernel; use Illuminate\Support\Arr; @@ -100,7 +101,7 @@ public function packageRegistered() if (config('nativephp-internal.running')) { $this->app->singleton( - \Illuminate\Contracts\Debug\ExceptionHandler::class, + ExceptionHandler::class, Handler::class ); diff --git a/src/System.php b/src/System.php index 7fc8104f..fa2d0c7e 100644 --- a/src/System.php +++ b/src/System.php @@ -44,7 +44,7 @@ public function decrypt(string $string): ?string } /** - * @return array<\Native\Desktop\DataObjects\Printer> + * @return array */ public function printers(): array { diff --git a/tests/ChildProcess/ChildProcessTest.php b/tests/ChildProcess/ChildProcessTest.php index c4a00c2f..7463e05e 100644 --- a/tests/ChildProcess/ChildProcessTest.php +++ b/tests/ChildProcess/ChildProcessTest.php @@ -144,3 +144,28 @@ ChildProcess::start('foo bar', 'some-alias'); Http::assertSent(fn (Request $request) => $request['persistent'] === false); }); + +it('can mark a process as handling its own shutdown', function () { + ChildProcess::start('foo bar', 'some-alias', handlesOwnShutdown: true); + Http::assertSent(fn (Request $request) => $request['handlesOwnShutdown'] === true); +}); + +it('can mark a php command as handling its own shutdown', function () { + ChildProcess::php("-r 'sleep(5);'", 'some-alias', handlesOwnShutdown: true); + Http::assertSent(fn (Request $request) => $request['handlesOwnShutdown'] === true); +}); + +it('can mark an artisan command as handling its own shutdown', function () { + ChildProcess::artisan('foo:bar', 'some-alias', handlesOwnShutdown: true); + Http::assertSent(fn (Request $request) => $request['handlesOwnShutdown'] === true); +}); + +it('can mark a node process as handling its own shutdown', function () { + ChildProcess::node('path/to/file.js', 'some-alias', handlesOwnShutdown: true); + Http::assertSent(fn (Request $request) => $request['handlesOwnShutdown'] === true); +}); + +it('does not handle its own shutdown by default', function () { + ChildProcess::start('foo bar', 'some-alias'); + Http::assertSent(fn (Request $request) => $request['handlesOwnShutdown'] === false); +}); diff --git a/tests/Fakes/FakeChildProcessTest.php b/tests/Fakes/FakeChildProcessTest.php index 80d3299d..0a62e550 100644 --- a/tests/Fakes/FakeChildProcessTest.php +++ b/tests/Fakes/FakeChildProcessTest.php @@ -76,6 +76,14 @@ $this->fail('Expected assertion to fail'); }); +it('asserts handlesOwnShutdown on a started process when it was set', function () { + swap(ChildProcessContract::class, $fake = app(ChildProcessFake::class)); + + $fake->start('cmd', 'alias', handlesOwnShutdown: true); + + $fake->assertStarted(fn ($cmd, $alias, $cwd, $env, $persistent, $handlesOwnShutdown) => $handlesOwnShutdown === true); +}); + it('asserts php using callable', function () { swap(ChildProcessContract::class, $fake = app(ChildProcessFake::class));