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

Lwt does not use Sys.set_signal #1012

Open
polytypic opened this issue Apr 15, 2024 · 0 comments
Open

Lwt does not use Sys.set_signal #1012

polytypic opened this issue Apr 15, 2024 · 0 comments

Comments

@polytypic
Copy link

polytypic commented Apr 15, 2024

The documention on signals says

reinstall_signal_handler signum if any signal handler is registered for this signal with Lwt_unix.on_signal, it reinstall the signal handler (with Sys.set_signal). This is useful in case another part of the program install another signal handler.

Perhaps I'm not reading this right, but this seems to say that reinstall_signal_handler signam uses Sys.set_signal to reinstall the signal handler used by Lwt.

Did I read correctly?

Because, if I did, then the documentation is wrong.

First of all, Sys.set_signal is only mentioned in the Lwt source code in this repository in the documentation comment and the related Sys.signal is not mentioned in the source code at all.

Looking at the code, Lwt seems to have its own native bindings for dealing with signals.

Finally, those native bindings do not seem to be compatible with Sys.set_signal and Sys.signal as demonstrated by this REPL interaction:

# #require "lwt" ;;
# #require "lwt.unix" ;;
# Sys.set_signal Sys.sigusr2 (Signal_handle ignore) ;;
- : unit = ()
# (* We can now see that we had a non default handler *)
  Sys.signal Sys.sigusr2 Signal_default ;;
- : Sys.signal_behavior = Sys.Signal_handle <fun>
# (* Indeed we did, let's then use Lwt *)
  Lwt_unix.on_signal Sys.sigusr2 ignore ;;
- : Lwt_unix.signal_handler_id = <abstr>
# (* We probably should now see a non default handler *)
  Sys.signal Sys.sigusr2 Signal_default ;;
- : Sys.signal_behavior = Sys.Signal_default
# (* Well, OK, let's reinstall, which should use Sys.set_signal *)
  Lwt_unix.reinstall_signal_handler Sys.sigusr2 ;;
- : unit = ()
# (* We should now see a non default handler *)
  Sys.signal Sys.sigusr2 Signal_default ;;
- : Sys.signal_behavior = Sys.Signal_default

Looking at git history, I see this commit.

Are the signal handling problems still relevant for Lwt?

(I've recently been using signals through the standard libraries and while there are know issues in 5.0.0, on 4.13, 4.14, and 5.1.1 I haven't (yet) seen issues with signals.)

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

No branches or pull requests

1 participant