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

Fix: Update Signal Handling for Unix-like and added Windows Systems support #76

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

yourchocomate
Copy link
Contributor

@yourchocomate yourchocomate commented Mar 13, 2024

laravel-reverb.mp4

Fix: #65
Fix: #74

Description
This PR enhances the signal handling mechanism to support both Unix-like systems and Windows. It introduces support for SIGTSTP on Unix-like systems and implements Windows-specific signal handling using sapi_windows_set_ctrl_handler. This update improves the platform compatibility of the application and ensures proper signal handling across different operating systems

Changes Made

  • Added SIGTSTP to the list of subscribed signals for Unix-like systems to not suspend process without gracefullyDisconnect the opened server
  • Introduced a new method handleSignalWindows() to handle signals on Windows.
  • Implemented Windows-specific signal handling using sapi_windows_set_ctrl_handler.

Testing

  • Tested on Unix-like systems to ensure correct handling of SIGTSTP.
  • Tested on Windows to verify proper signal handling using sapi_windows_set_ctrl_handler.

Any feedback or suggestions for improvement are welcome.

@luisprmat
Copy link

image

This is the issue in windows that this PR solves.
It worked for me, but making that modification.

@driesvints driesvints merged commit b5c62ba into laravel:main Mar 14, 2024
11 checks passed
@mfrizky
Copy link

mfrizky commented Mar 27, 2024

I still get the same error in docker environment with php 8.3.4 fpm-bookworm in my development server. This problem doesnt occure to me on my local machine with windows OS non docker environment.

@yourchocomate
Copy link
Contributor Author

I still get the same error in docker environment with php 8.3.4 fpm-bookworm in my development server. This problem doesnt occure to me on my local machine with windows OS non docker environment.

I will check it out, Can you provide the docker config you have used?

@mfrizky
Copy link

mfrizky commented Mar 28, 2024

Sure, here are my docker-compose.yml and Dockerfile of my php :
Dockerfile.txt
docker compose yml.txt

Update :
my lead come up with a solution to use pcntl in Dockerfile like this
RUN docker-php-ext-configure pcntl --enable-pcntl \ && docker-php-ext-install pcntl
the reference probably from this site :
https://laracasts.com/discuss/channels/laravel/reverb-installation-issue?page=1&replyId=930286

Thank you and sorry for the trouble 🙏

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

Successfully merging this pull request may close these issues.

Undefined constant "Laravel\Reverb\Servers\Reverb\Console\Commands\SIGINT" Windows not supported (yet)?
4 participants