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

[Aaron Puchert]

aaronpuchert added inline comments.

INLINE COMMENTS

> core.cpp:58
> +static int signalPipeWrite = -1;
> +static volatile std::sig_atomic_t signalReceived = 0;
> +static QSocketNotifier* signalNotifier = nullptr;

I think it's better to use `std::atomic` here, `volatile` should be a thing of the past.

> core.cpp:68-69
>  
>      if ( !handlingSignal ) {
>          handlingSignal = 1;
> +        qCWarning(SHELL) << "Going down on signal" << sig;

Is this supposed to be thread-safe? Because it isn't. If it should be thread-safe, make `handlingSignal` a `std::atomic` (non-volatile) and use `compare_exchange_strong`.

> core.cpp:79-83
> +    signalNotifier->setEnabled(false);
> +
>      // re-raise signal with default handler and trigger program termination
>      std::signal(sig, SIG_DFL);
>      std::raise(sig);

Shouldn't this be in the `if` as well or what am I missing?

> core.cpp:86
>  
> +void signalHandler(int sig)
> +{

If I read the standard correctly, this needs C linkage, so wrap it in `extern "C" {...}`.

> core.cpp:90
> +    if (signalPipeWrite != -1) {
> +        qCDebug(SHELL) << "signal" << sig << " received, shutting down gracefully";
> +        write(signalPipeWrite, &sig, sizeof(sig));

Can you use this in a signal handler?

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/20181015/1d7f9c99/attachment-0001.html>