[Libreoffice-commits] core.git: coverity#1323754 we apparently can survive std::abort for a while
sbergman at redhat.com
Fri Sep 11 07:04:43 PDT 2015
On 09/11/2015 12:34 PM, Michael Meeks wrote:
> Good question; it may well be guaranteed - but - seemingly I saw this
> code-path continue; perhaps this is an artifact of the debugger under
> has some more details; but I'd swear to not having pressed ignore in my
> cases either so ... ;->
>> So, like Coverity, I fail to
>> see how that line can ever be reached (and bAbortFired, of automatic
>> storage during in OpenGLWatchdogThread::execute, ever be true).
> =) well, me too - was gob-smacked etc. of course, in the ideal world
> this is true; perhaps I was just gotcha'd by the debugging environment.
But I doubt we want to make our code base more capricious than
necessary, to shield us from behavior exhibited by the Windows debugging
> Then again - during our abort handling - we spend a lot of time
> creating GUI dialogs and so on on the main thread (which is by now this
> one) - that could easily also wedge / lock-up ;-) that's particularly
> true wrt. the problem of getting the solar-mutex; my hope is that the
> abort handler is good with dropping that.
> Which thread would you expect the signal to be delivered to (I wonder)
> - it's all a bit interesting I suspect.
The case should be pretty clear for a synchronous, std::abort-generated
SIGABRT (hopefully even on Windows).
> My hope was that the watchdog would carry on working in these cases &
> kill us again more aggressively if necessary if people insist on
> ignoring these guys.
But how should it do that? Even if the SIGABRT-handling were done on
another thread, the watchdog thread just couldn't progress past the
std::abort() (notwithstanding cheating in a debugging environment).
>> really wanted to do is make bAbortFired static, and set it to true
>> /before/ calling std::abort()?
> I guess we could launch another watchdog thread in this case (if indeed
> we believe the that std::abort never returns ;-) in which case making
> that static would be useful indeed. Would love to see a patch like that.
So there's only a single instance of the watchdog thread supposed to
ever run. The odd "static bool bFired" in OpenGLWatchdogThrad::execute
had fooled me to assume otherwise (for why else should the variable have
static storage duration).
Anyway, generalizing that "watchdog the OpenGLWatchdogThread, in case
our signal handler gets stuck" idea obviously leads to a "watchdog our
signal handler, in case it gets stuck" feature, i.e., spawn a thread
early in our signal handler (assuming spawning an additional thread
doesn't make our violation of what a signal handler is supposed to be
allowed to do any worse), which will call _exit after a fixed amount of
time. The question just is, what is a reasonable value for that amount
of time. Make it too short, and you'll prevent recovery of documents
that take long to save and for which our document recovery would
otherwise have happened to work fine.
And the true route ahead of course is to no longer put our document
recovery strategy at the mercy of a brittle, undefined-behavior--riddled
More information about the LibreOffice