Skip to content

Improve shutdown for FwLiteWeb in Paratext #1992

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Improve shutdown for FwLiteWeb in Paratext #1992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
525d58c
Improve shutdown handling for FwLiteWeb
hahn-kev Sep 9, 2025
032eea7
never throw on shutdown, just log the error and return false
hahn-kev Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions backend/FwLite/FwLiteWeb/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
using Microsoft.Extensions.Options;

//paratext won't let us change the working directory, and if it's not set correctly then loading js files doesn't work
Directory.SetCurrentDirectory(Path.GetDirectoryName(typeof(Program).Assembly.Location)!);

Check warning on line 6 in backend/FwLite/FwLiteWeb/Program.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Linux 

'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.

Check warning on line 6 in backend/FwLite/FwLiteWeb/Program.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Linux 

'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.
var app = FwLiteWebServer.SetupAppServer(new() {Args = args});
await using (app)
{
Expand All @@ -15,6 +15,14 @@
var url = app.Urls.First();
LocalAppLauncher.LaunchBrowser(url);
}
//windows is dumb and so you can't send SIGINT to a process, so we need to listen for a shutdown command
_ = Task.Run(async () =>
{
// Wait for the "shutdown" command from stdin
while (await Console.In.ReadLineAsync() is not "shutdown") { }

await app.StopAsync();
});
Comment on lines +18 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Handle EOF and be resilient to whitespace/case; avoid a potential busy loop on closed stdin.

If stdin closes (parent dies or doesn’t pipe input), ReadLineAsync() returns null and the current loop spins forever without calling StopAsync(). Also, be lenient to trailing whitespace/casing.

Apply this diff:

-    //windows is dumb and so you can't send SIGINT to a process, so we need to listen for a shutdown command
-    _ = Task.Run(async () =>
-         {
-             // Wait for the "shutdown" command from stdin
-             while (await Console.In.ReadLineAsync() is not "shutdown") { }
-
-             await app.StopAsync();
-         });
+    // Windows can't receive SIGINT as a child; listen for a "shutdown" command on stdin.
+    _ = Task.Run(async () =>
+    {
+        string? line;
+        while ((line = await Console.In.ReadLineAsync()) is not null
+               && !string.Equals(line.Trim(), "shutdown", StringComparison.OrdinalIgnoreCase))
+        { /* keep waiting */ }
+
+        await app.StopAsync();
+    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//windows is dumb and so you can't send SIGINT to a process, so we need to listen for a shutdown command
_ = Task.Run(async () =>
{
// Wait for the "shutdown" command from stdin
while (await Console.In.ReadLineAsync() is not "shutdown") { }
await app.StopAsync();
});
// Windows can't receive SIGINT as a child; listen for a "shutdown" command on stdin.
_ = Task.Run(async () =>
{
string? line;
while ((line = await Console.In.ReadLineAsync()) is not null
&& !string.Equals(line.Trim(), "shutdown", StringComparison.OrdinalIgnoreCase))
{ /* keep waiting */ }
await app.StopAsync();
});


await app.WaitForShutdownAsync();
}
29 changes: 26 additions & 3 deletions platform.bible-extension/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ export async function activate(context: ExecutionActivationContext): Promise<voi
// Services
await entryService,
// Other cleanup
() => fwLiteProcess?.kill(),
() => shutdownFwLite(fwLiteProcess),
);

logger.info('FieldWorks Lite is finished activating!');
Expand All @@ -275,8 +275,7 @@ function launchFwLiteFwLiteWeb(context: ExecutionActivationContext) {
context.executionToken,
binaryPath,
['--urls', baseUrl, '--FwLiteWeb:OpenBrowser=false', '--FwLiteWeb:CorsAllowAny=true'],
// eslint-disable-next-line no-null/no-null
{ stdio: [null, 'pipe', 'pipe'] },
{ stdio: ['pipe', 'pipe', 'pipe'] },
);
fwLiteProcess.once('exit', (code, signal) => {
logger.info(`[FwLiteWeb]: exited with code '${code}', signal '${signal}'`);
Expand All @@ -293,3 +292,27 @@ function launchFwLiteFwLiteWeb(context: ExecutionActivationContext) {
}
return { baseUrl, fwLiteProcess };
}

function shutdownFwLite(fwLiteProcess: ReturnType<typeof launchFwLiteFwLiteWeb>['fwLiteProcess']): Promise<boolean> {
return new Promise((resolve, _) => {
fwLiteProcess.once('exit', (code, signal) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this make the fwLiteProcess.once('exit', ... in lines 280-282 unnecessary?

if (code === 0) {
resolve(true);
} else {
logger.error(`[FwLiteWeb]: shutdown failed with code '${code}', signal '${signal}'`);
resolve(false);
}
});
fwLiteProcess.once('error', (error) => {
logger.error(`[FwLiteWeb]: shutdown failed with error: ${error}`);
resolve(false);
});

fwLiteProcess.stdin.write('shutdown\n');
fwLiteProcess.stdin.end();
setTimeout(() => {
fwLiteProcess.kill('SIGKILL');
resolve(true);
}, 10000);
});
}
Loading