[Development] Disavowing the Lakos Rule for Q_ASSERT

[Marc Mutz]

Nothing that you describe in the context of a bug in our code here is 
specific to throwing precondition checkers. If you weren't checking with 
active precondition handlers (the status quo in release mode atm), then 
you'd likely would have run into language UB on precondition violation, 
with equally non-recoverable outcome (and the additional problem of 
language UB of time travel). If you throw, it may not be as safe a 
abort() on failed assertion, but safer than current release mode, 
because a) you eliminate time travel and b) likely will hit some catch 
or noexcept contexts that deterministically take the process down.

I note that on glib systems, at least, C code should support stack unwinding, because that's the way Posix thread cancellation is implemented there. On other platform, that means the callbacks should be markd as noexcept, then, or be wrapped in an exception-eating wrapper.

So I fail to see how throwing precondition handlers, siting, security-wise, 
smack between the Q_ASSERT and unchecked UB modes we already support 
right _now_, could be any less acceptable than these polar ends? It's 
just a new grey gradient stop between the current black and white ones 
we already have.

Thanks,
Marc

On 06.09.24 22:44, Thiago Macieira wrote:
> On Friday 6 September 2024 10:55:47 CEST Ville Voutilainen wrote:
>>> I'm arguing that (c) makes it *worse* because now the exception propagates
>>> out, causing all sorts of code to be run that we've never, ever tested
>>> before.
>> I would be rather surprised if you had tested that code before,
>> because it's my application
>> code where that exception propagates, not Qt framework code.
> 
> I meant the Qt code that the exception propagates through before reaching
> yours. I'm thinking of things like an accidental violation of a QList in one
> of the lists in QObjectPrivate::extraData, like runningTimers. Suppose the
> main() function puts the precondition checker in throw mode and that we do
> have a bug somewhere that triggers this violation deep inside servicing
> timers.
> 
> The exception will get thrown instead of the application aborting, but then
> it'll propagate through Qt code that has never been tested for exceptions. I
> argue that your best bet here is that it attempts to propagate through Win32
> DLLs or some Cocoa or Glib native APIs, which cause it to crash.
> 
> Because the alternative is that it reaches an exception handler somewhere and
> the user code may attempt to do something while the QObject and event
> dispatcher state is corrupt:
> 
>>> It may also be caught somewhere and execution incorrectly resumed, causing
>>> incorrect-state execution to continue.
>>
>> There's nothing incorrect about that; the exception will be caught and
>> handled, and application execution
>> will resume.
> 
> See above. In the scenario I outlined, you cannot trust anything that uses
> QObject any more.
> 
> And worse than crashing it is continuing to run with corrupted state. It may
> freeze or have weird, user-visible behaviours. Or it may corrupt user data,
> which then may be saved to disk, making it permanent and the data error not
> getting caught.
> 
>>> My argument is "first, do no harm" (or "don't make it worse"). Exceptions
>>> should only be used if there's a proper correction code for it.
>>
>> I have that "correction code".
> 
> Sure, but "bugs happen". I am arguing that putting the contract checker into
> exception mode expands the blast radius of when those bugs do occur.
> 
> 
-- 
Marc Mutz <marc.mutz at qt.io> (he/his)
Principal Software Engineer

The Qt Company
Erich-Thilo-Str. 10 12489
Berlin, Germany
www.qt.io

Geschäftsführer: Mika Pälsi, Juha Varelius, Jouni Lintunen
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht Charlottenburg,
HRB 144331 B