D16218: [KDevelop/Core]: safe signal-handler implementation (WIP)

[Aaron Puchert]

aaronpuchert added a comment.


  You're right, my remarks apply to the existing code already.
  
  In a POSIX world we could just block the signals and poll <https://linux.die.net/man/2/sigwaitinfo> for them in the main loop instead of writing these insane signal handlers, but our standard writers didn't want to leave room for that apparently.

INLINE COMMENTS

> rjvbb wrote in core.cpp:58
> From what I've read std::atomic is safe only if its `is_lock_free` property is true. If not, sig_atomic_t is the type of choice. (You can't lock a mutex in a signal handler.)
> 
> As to the volatile property, I just copied that from `handlingSignal`.

At least `std::atomic<bool>` is lock-free on all relevant architectures. (I'm aware that the standard doesn't guarantee it, probably because of some embedded architectures. With C++17 we could use std::atomic::is_always_lock_free in a static_assert.)

> rjvbb wrote in core.cpp:68-69
> I don't think there's a need for that.

Either two signals can be handled at the same time, then it needs to be thread-safe, or there is always only one signal handled at the same time, in which case this `if` isn't needed.

> rjvbb wrote in core.cpp:79-83
> No, the shutdown procedure seems to be designed to be a 2-stage process. The first interrupt will trigger a regular shutdown procedure (inside the if) and lead to exit if all goes well. If that process blocks or if another signal comes in before it can terminate, the part outside the if is executed.
> 
> In its current form you could indeed put the SIG_DFL statement inside or before the if. In my original proposal there was a small addition here: shutdown of just the core plugin, evidently before restoring the default signal handler. That change will be resubmitted so I'd rather not move statements now that I'd have to move back again.

> The first interrupt will trigger a regular shutdown procedure (inside the if) and lead to exit if all goes well. If that process blocks or if another signal comes in before it can terminate, the part outside the if is executed.

But why? If I want an unclean shutdown, I can just send SIGKILL. I don't understand why a second signal should be handled differently than the first.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D16218

To: rjvbb, #kdevelop, kfunk
Cc: aaronpuchert, brauch, kfunk, arrowd, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20181016/18170b80/attachment.html>