Skip to content

feat: send_signal subroutine in system module #947

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

Closed
wants to merge 4 commits into from

Conversation

wassup05
Copy link
Contributor

@wassup05 wassup05 commented Mar 8, 2025

This PR proposes a send_signal subroutine to the system module.

  • Can be used to send any POSIX signal to a process (Ex: SIGKILL in which case kill subroutine does the job)
  • accepts signal as an integer argument representing the signal number
  • returns a boolean flag success which is true if all went well and false if some errors ocurred
  • updates the state of the process object
  • It does nothing on Windows as the concept of signals is not really present there

@perazz
Copy link
Member

perazz commented Mar 9, 2025

Thank you for your contribution @wassup05. I appreciate the effort in expanding the process module.

I have some concerns regarding the inclusion of a send_signal function and how would that pair with current functionality (i.e. the kill function and the process_type class). Here is some thoughts:

  • Signals are a low-level POSIX mechanism, and there is no direct equivalent on Windows. I would be cautious implementing a low-level OS function that only works on Unix-like systems, the philosophy is to mostly hide the OS as much as possible., and have a fully cross-platform API;
  • AFAICT there is no way to achieve a Windows equivalent (TerminateProcess or GenerateConsoleCtrlEvent?)
  • What further functionality could this achieve beyond killing a process? As we already have cross-platform kill(process), maybe we should think of a similar high-level API for other signal-based actions instead.

Looking forward to hearing your thoughts as well as those of the other maintainers, @jalvesz @jvdp1?

@wassup05
Copy link
Contributor Author

wassup05 commented Mar 9, 2025

Thank you @perazz for your reply, Here is what I think about the concerns you have raised

  • I think if we just limit ourselves to cross-platform stuff, as in the things that are common between the two operating systems we would miss out on a lot of useful functionality, of course I am not asking for the inclusion of the complete POSIX API or the Windows API but having a few frequently used functions would be quite useful imo
  • Yes that is true, Windows mostly communicate through messages like WM_CLOSE etc and TerminateProcess and GenerateConsoleControlEvent are the only ones I know of, maybe send_signal could call TerminateProcess on Windows?
  • the "problem" is that kill just instanly terminates a process, no questions asked, the process cannot handle it any way, but signals like SIGTERM,SIGPIPE,SIGUSR1,SIGUSR2,SIGHUP have a general use for them and can be handled by a process... for example: let's say there is a process running which is managing a database, if you kill it, it may not clean up stuff properly, some data maybe lost in the process but sending a SIGTERM would ask the process to graciously terminate and it can do all the necessary cleanup before exiting. And not all signals cause a process to exist, like SIGHUP is used for reloading a configuration and so on.

@jalvesz
Copy link
Contributor

jalvesz commented Mar 11, 2025

Thanks @wassup05 for this PR,

I have not much experience with low level signal management, so I will refrain from giving opinions on the technical aspect.

I agree with @perazz regarding the fact that stdlib tries as hard as possible to be agnostic to the platform and to propose an unified API for all users. So this should be kept in mind as much as possible. Non cross-platforms APIs should be the exception.

I do see an interest in the last point mentioned by @wassup05. I would advise that this debate is opened at discourse following a similar spirit as the thread opened by @perazz here https://fortran-lang.discourse.group/t/stdlib-system-interaction-api-call-for-feedback/9037 in order to collect the opinions of the wider community.

@perazz
Copy link
Member

perazz commented Mar 12, 2025

To add to the discussion, here is two prototype subroutine signatures that I think would be viable for stdlib, and that are in line with the current kill API:

  ! Send SIGINT or equivalent in Windows
  subroutine interrupt(process) 
    type(process_type), intent(inout) :: process
  end subroutine interrupt

  ! Send SIGTERM or equivalent on Windows
  subroutine terminate(process)
    type(process_type), intent(inout) :: process
  end subroutine terminate

Let me stress that cross-platform functionality would be a necessary feature imho.

@arjenmarkus
Copy link
Member

arjenmarkus commented Mar 12, 2025 via email

@wassup05
Copy link
Contributor Author

wassup05 commented Mar 12, 2025

To add to the discussion, here is two prototype subroutine signatures that I think would be viable for stdlib, and that are in line with the current kill API:

  ! Send SIGINT or equivalent in Windows
  subroutine interrupt(process) 
    type(process_type), intent(inout) :: process
  end subroutine interrupt

  ! Send SIGTERM or equivalent on Windows
  subroutine terminate(process)
    type(process_type), intent(inout) :: process
  end subroutine terminate

Let me stress that cross-platform functionality would be a necessary feature imho.

@perazz I agree that this indeed is a good option but I think there will be some problems on the Windows side, because generating a SIGINT would require GenerateConsoleCtrlEvent() and this apparently only works if the process sending the signal is attached to the same console as the process receiving the signal, this requires first of all that the process be a Console process and not a GUI and hence the way is not really uniform

SIGTERM is not natively raised on Windows (reference) and trying to simulate it using GenerateConsoleCtrlEvent() again has the same limitations

@perazz
Copy link
Member

perazz commented Mar 17, 2025

Thanks for the comments @wassup05. Imho this would be a limiting factor for a Fortran library that strives to be system-agnostic. I would be happy to see possible solutions that would make this work also on Windows OSses.

@wassup05
Copy link
Contributor Author

I realised I missed a crucial point while thinking about this (I am sorry)... on the C side you could send signals to random pid but because of the type process here, there are limitations.. you must have created the process using the same process variable... which simplifies stuff on the windows side because these are indeed attached to the same console

So I was thinking, send_signal could do the usual on unix like systems (i.e send the requested signal) but on windows side if the signal is SIGINT we could send a CTRL_C_EVENT or CTRL_BREAK_EVENT and if it is SIGTERM we could send CTRL_CLOSE_EVENT and if any other signal is provided, since there are no equivalent on Windows we could just try to Terminate the Process.... hence providing alternatives to both kinds of users

Or as suggested by @perazz, just interrupt and terminate also can be a good option (maybe compromising the needs of unix users a bit)

Which one do you think is a good idea? If at all you deem this is a useful and worthwhile functionality to have in the system module (I would like honest remarks on this too!)

@perazz @jalvesz @arjenmarkus

@arjenmarkus
Copy link
Member

arjenmarkus commented Mar 18, 2025 via email

@perazz
Copy link
Member

perazz commented Mar 21, 2025

I tend to agree with @arjenmarkus. My opinion is that the practical way to implement that is to have two more functions interrupt and terminate (or equivalent names). Internally, they will wrap calls to send signals on Unix, and the equivalent CTRL_BREAK_EVENT and CTRL_CLOSE_EVENT on Windows. This is the way system-based functionality is usually wrapped in Fortran.

@wassup05
Copy link
Contributor Author

Okay thank you @perazz and @arjenmarkus for your suggestions. I will make another PR regarding the same and close this as this is no longer necessary.

@wassup05 wassup05 closed this Mar 21, 2025
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.

4 participants