Skip to content
This repository was archived by the owner on Jul 4, 2025. It is now read-only.

Conversation

@ohaiibuzzle
Copy link
Contributor

Describe Your Changes

This mainly adds some nice QoL feature for Cortex Server when it is being ran on its own (ala. directly executing the cortex-server binary):

  • Include a help output on -h or --help for the available options
  • Web server now run in a separate detached thread instead of blocking the main thread.
  • Proper SIGINT handling for gracefully shutting down the web server.

Fixes Issues

  • None, this is just pure QoL.

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

@ohaiibuzzle ohaiibuzzle force-pushed the ohaiibuzzle/feat/server_sigint_handler branch 2 times, most recently from 3f973fa to c89618c Compare February 5, 2025 14:26
@louis-jan
Copy link
Contributor

Awesome!

@ohaiibuzzle ohaiibuzzle force-pushed the ohaiibuzzle/feat/server_sigint_handler branch from c89618c to 696af62 Compare February 6, 2025 03:20
@vansangpfiev
Copy link
Contributor

Hi @ohaiibuzzle, we encountered an issue before with Ctrl + C on Windows when running cortex CLI: it also terminated the cortex Server process. A workaround for this is to ignore Ctrl + C event on the Server side and use /destroy endpoint to stop the Server (yes - it is tricky and ugly).

Can you please help to check the PR on the Windows?

@ohaiibuzzle
Copy link
Contributor Author

ohaiibuzzle commented Feb 7, 2025

@vansangpfiev I will check for that later, I only tested on macOS and Linux.

Although, afaik, in Windows console properties are shared by all processes sharing that same console host instance, so what likely happened is when that Ctrl handler is installed, it becomes global for the console host instance shared by cortex and cortex-server.

So even when cortex-server moved to background it still maintains itself as the Ctrl C handler.

As for the solution: This on the CLI should work: https://stackoverflow.com/a/71364777

@ohaiibuzzle
Copy link
Contributor Author

ohaiibuzzle commented Feb 7, 2025

Update: I compiled it on my Windows system. Could not reproduce. Even Ctrl+C have the server instance starts up correctly in the background. Perhaps I am missing something...?

Yes, I managed to reproduce it. Fixed by setting CREATE_NO_WINDOW to CreateProcess which detaches the server child completely from the parent CLI: https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags

@ohaiibuzzle ohaiibuzzle force-pushed the ohaiibuzzle/feat/server_sigint_handler branch from 6d660e5 to e870bdb Compare February 7, 2025 20:24
@vansangpfiev vansangpfiev merged commit 4132f2f into janhq:dev Feb 8, 2025
8 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants