Subject: bug#46009: exception from inside false-if-exception?

From: Maxime Devos <[email-masked]>
Date: Mon, 29 Apr 2024 16:13:22 +0200

[Adding Andy Wingo because of the stack shenanigans]

>Subject: exception from inside false-if-exception?

Duplicate of #46009 - (backtrace) crash, string->number: Wrong type argument in position 1 (expecting string): #f - GNU bug report logs

>the expression pointed to by debug.scm,72:40 is this:

>(false-if-exception (string->number (getenv "COLUMNS")))

All uses of false-if-exception are wrong – I haven’t found a single exception (pun intended) to this rule so far.  It shouldn’t be treating stack overflows, out-of-memory, exceptions from asyncs, EPERM, etc., as #false.  – false-if-exception delenda est

[...]

What I think is going on here, is that the exception catching API is a bit of mess, and to a degree, non-composable, leading to bugs like this.

1. raise/raise-continuable + guard: perfectly composable, no problems.
2. throw + catch: that’s fine too, it’s just a historical Guile API for (1), albeit slightly less general. 
3. with-exception-handler – a procedural variant of the ‘guard’ macro – sure ok. Also good for continuable exceptions, IIUC.
4. Pre-unwind handlers / with-throw-handler. / ???.

This is the non-composable stuff (or, difficult to understand and compose, at least). If you are catching an exception to _handle_ it (say, in this case, with false-if-exception), then any throw handler/pre-unwind handler/whatever has no business whatsoever to interfere, and neither to even know something is happening in the first place.

Yet, the documentation says it “is used to be able to intercept an exception that is being thrown before the stack is unwound” – which it has no business of doing in the first place (non-composable). 

Also the description of pre-unwind handlers / with-throw-handler is rather low-level (it’s more described in terms of (un)winding rather than nesting) and it appears to have been grown a bit .. organically, I think it’s time for it to be replaced by a new design that’s (conceptually) closer to raise/guard/...

I suspect the problem is that these throw handlers or whatever are messing things up – whether because they are hard to use, impossible to use correctly or because they are incorrectly implemented in Guile and would propose them to be replaced by something else.

On this something else: according to the documentation, these throw handlers can be used for:

1. Clean up some related state
2. Print a backtrace
3. Pass information about the exception to a debugger

1: IIUC, this is what (dynamic-wind #false thunk clean-up-stuff) is for – this is not entirely correct w.r.t. userspace threading implementation, but can be salvaged with something like

>https://github.com/wingo/fibers/blob/7e29729db7c023c346bc88c859405c978131f27a/fibers/scheduler.scm#L278

to override ‘dynamic-wind’ (actually I think the API ‘rewinding-for-scheduling?’ would need to be adjusted a bit to allow for situations like implementing threads inside threads inside threads inside ..., but it should give the basic idea.)

2. the rationale for this reason, is that when an exception is caught (think ‘catch’ handler), we are not in the same dynamic environment anymore, so by then it’s too late to generate a backtrace. But we can have a cake(= have a backtrace) and eat it too(= post-unwind / in the catch handler), by using ‘with-exception-handler’ with ‘#:unwind? #false’.

That’s not entirely sufficient, because sometimes the exception was already caught and ‘wapped’ to give some extra context in the exception object (but losing some backtrace in the process). OTOH, this ‘losing some backtrace’ already happens in the current implementation IIUC, so it wouldn’t be worse than the current implementation in this respect. I think there is a solution that isn’t just “always record the backtrace and put it in the exception” (which would be terribly inefficient in situation you mentioned), but I haven’t found it yet.

Perhaps the (full) current continuation (think call/cc) could be saved? And then later, when ultimately a backtrace is to be printed, the stack of this continuation can be investigated – This might seem even more inefficient than saving a backtrace, but if you think about it, all those frames were already allocated, so you don’t need to allocate new memory or do much CPU things beyond saving some pointers, you just need to start some new memory branching of an earlier point in the dynamic environment/the frames when/after rewinding.  I’m thinking of spaghetti and cactus stacks.   (The benefit beyond ‘just include backtrace’ is: (1) almost 0 time/memory cost when not used (2) if desired, you can print _more_ information than just the backtrace, e.g. values of certain parameters or whatever.)

And after the backtrace or whatever has been printed, there is no reference to the call/cc thing anymore so it can be garbage collected(*).

(*) there is a bit of a caveat here with respect to user-level threading and pausing computations, where some stuff low(high?) on the stack might be saved for too long, so perhaps it would instead be better to save a delimited continuation – the exception handler that does the backtracing could then set some parameter with a tag that delimits where to stop the delimited continuation – for a comparison: I think there is a procedure with a name like “call-with-stack” or something close to that which does something like that.

(Also, the section “Exceptions” isn’t mentioning ‘raise + guard’, instead there is a separate R6RS section isolated from the rest of the manual – it treats historical Guile idiosyncrasies as the norm and standard SRFI / RnRS as an aberration. But that’s a different thing.)