Handle SIGTERM as SIGINT and ignore SIGHUP

[DimitriPapadopoulos]

Not sure about this one:

• Why do we avoid calling pthread_sigmask() on MacOS X to disable signal handling in future threads?
• Why are we using C's signal() instead of POSIX sigaction()?

See for example:

• What is the difference between sigaction and signal?

Fixes #78.

[Mabin-J]

In macOS, I got your branch and tested after remove #132 but.. Ctrl + C didn't works...

And...
openfortivpn can be killed with kill pid command but with this PR, it cannot be killed same command.
it was killed with kill -9 pid...

[DimitriPapadopoulos]

@Mabin-J Unfortunately I cannot test on MacOS, however:

• As far as I know Ctrl + C generates a SIGINT signal on MacOS, same as on Linux.
• The patch shouldn't change the way SIGINT is handled, it just adds SIGTERM.

So unless I am mistaken this patch shouldn't change Ctrl + C handling at all. Why have you removed #132? Doesn't #132 fix Ctrl + C handling on MacOS?

[DimitriPapadopoulos]

@Mabin-J About kill pid:

• By default kill sends signal 15 (SIGTERM).
• With my patch signal SIGTERM is handled the same way as SIGINT.
• As far as I can understand SIGINT is not handled properly without fix lock status when fail to spawn pppd in macOS. #132, so I'm not surprised if SIGTERM is not handled properly without fix lock status when fail to spawn pppd in macOS. #132 either.

What happens with both #134 and #132 applied?

[Mabin-J]

@DimitriPapadopoulos,
I think that this PR purpose is finishing any situation.
So I tested remove #132.

and... when openfortivpn connect normally, Ctrl + C already worked without this PR...

[DimitriPapadopoulos]

This PR is not about changing handling of Ctrl + C, so I do hope Ctrl + C works the same with or without it.

This PR is for consistent handling of SIGTERM and SIGINT, and for ignoring SIGHUP.

[Mabin-J]

@DimitriPapadopoulos
Sorry. I understand that this PR's purpose after read your comment and test more situation.

Before this PR, openfortivpn connected normally and run kill pid, It can be finish but without destroy process.
But after thisPR, kill pid can be finish with destroy process.
Right?

It works correctly in macOS. :)

[DimitriPapadopoulos]

@Mabin-J Thanks for testing this PR on MacOS.

I'm still not sure why we avoid calling pthread_sigmask() on MacOS X to disable signal handling in future threads, but that's probably best dealt with in a different issue.

[DimitriPapadopoulos]

@Mabin-J Indeed this PR should allow for consistent and proper clean-up afterkill pid (SIGTERM) just like after Ctrl + C (SIGINT).

Proper clean-up depends on the signal handler and the related logic of the application. We might need to review and change the current code in openfortivpn, especially for MacOS, but that's a different issue and at least kill pid (SIGTERM) and Ctrl + C (SIGINT) are handled consistently.

[Mabin-J]

@DimitriPapadopoulos, I understood.

[mrbaseman]

Sorry if I repeat things that have already been said...

@DimitriPapadopoulos to your question "Why do we avoid calling pthread_sigmask() on MacOS X to disable signal handling in future threads?" I just can say that this was part of the initial MacOS X port fretn@382fefd to which @fretn had added the documentation and which I rebased and squashed end of last year.

If this part of the MacOS X code can be improved, please go ahead. Maybe the #ifndef __APPLE__ (io.c#L577) and #endif (io.c#L584) can just be dropped now?

We have seen yesterday in #132, that openfortivpn works on MacOS X with this part of the signal handling, but it could not be canceled with Ctrl + C anymore in normal operation (actually, that's what the comment on io.c#L578 gives as an explanation. @Mabin-J could you repeat this test with the patches of #134 applied? If it works fine, we can drop these two preprocessor lines together with the current PR, but if signal handling on MacOS X needs further investigation I would vote for a separate issue to address this.

[Mabin-J]

@mrbaseman, Sorry. I forgot to test that your request. I'll test it tomorrow.

[Mabin-J]

I tested some case.

• this PR

  • It ran correctly.

• this PR after remove io.c#L577, io.c#L584

  • it wasn't finished after I pushed Ctrl + C when it connect normally.
  • it was only finished after I ran killall -9 openfortivpn.
  • it seems that io.c#L577, io.c#L584 is needed.

• this PR without fix lock status when fail to spawn pppd in macOS. #132 (I know that this PR isn't related with fix lock status when fail to spawn pppd in macOS. #132, just test)

  • WARN: read returned 0 was repeated infinitely... :(
  • but pthread_cancel issue is solved, we don't care about this.