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

[René J.V. Bertin]

rjvbb added a comment.


  >   Arguably that still stands, but it's outside my area of expertise to comment on it.
  
  FWIW, I took Kevin's remark about this as a reaction to the fact that I moved the signal handling stuff into the CorePrivate class, and a request to take it back out as it was before. I think his other remarks in the exchange we had mean that he didn't intend me to write a dedicated signal handling class. I still could of course, but is it really worth the effort given that there's unlikely to be reuse and the only reason to instantiate the class would be to have a target to connect to?
  
  >> core.cpp:103
  >>  +        write(signalPipeWrite, &sig, sizeof(sig));
  >>  +    }
  >>  +}
  > 
  > Maybe you can add an else branch where you re-raise the signal with the default handler.
  
  If the goal is just to let a 2nd signal trigger the default reaction from the runtime then it can be even simpler. No need for an else, a `signal(sig, SIG_DFL)` after that write() would do the same thing.
  But remember there was a 2nd aspect to this patch initially which I've been told to submit in a separate patch: handle SIGHUP with a more basic shutdown method, the same as used in case a second signal is handled.
  
  Of course that could also be done in a different way: connect the QSocketNotifier signal to a different slot ("shutDownSafelyNow()") before attempting the graceful exit (and call that slot when a SIGHUP arrives):
  
    shutDownSafelyNow()
    {
        std::signal(sig, SIG_DFL);
    
        if (corePrivateInstance->m_core) {
           // shutdown core functionality, in particular the DUChain subsystem
           // in an effort to prevent cache corruption. It's only cache, but
           // regenerating it can be very time-consuming.
           corePrivateInstance->m_core->shutdown();
        }
    
        signalNotifier->setEnabled(false);
    
        // re-raise signal with default handler and trigger program termination
        std::raise(sig);
    }

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kfunk, aaronpuchert
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/20181020/57845e97/attachment.html>