exception from inside false-if-exception?

exception from inside false-if-exception?

[Attila Lendvai]

dear fellow Guilers,

context:
--------

i'm working on shepherd (with several non-trivial local commits). its test suite runs clean from a shell, but fails when i try to `./pre-inst-env guix build -K shepherd@0.10.99-git`.


the sympthom:
-------------

COLUMNS is not set in the guix build env, and the basic.sh test fails with the following exception/backtrace in the test log:

[...]
In ice-9/boot-9.scm:
  1747:15  8 (with-exception-handler [...])
In shepherd/support.scm:
    613:9  7 (_ . _)
In unknown file:
           6 (display-backtrace [...])
In system/repl/debug.scm:
   148:36  5 (print-frames [...])
In ice-9/boot-9.scm:
   2137:6  4 (_)
  1747:15  3 (with-exception-handler [...])
In system/repl/debug.scm:
    72:40  2 (_)
In ice-9/boot-9.scm:
  1685:16  1 (raise-exception _ #:continuable? _)
  1683:16  0 (raise-exception _ #:continuable? _)
ice-9/boot-9.scm:1683:16: In procedure raise-exception:
In procedure string->number: Wrong type argument in position 1 (expecting string): #f

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

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

if i paste this into a guile repl, then it behaves as expected.

i have verified that if i set COLUMNS in the basic.sh test, even if i set it to:

COLUMNS=""
export COLUMNS

then the guix package builds fine.

if i add:

unset COLUMNS

to the basic.sh test, then it makes it fail even in my dev shell (after a couple of minutes long timeout):

`make check TESTS="tests/basic.sh"`

there are no WITH-THROW-HANDLER's involved.


my question:
------------

unless i missed something, i seem to be getting an exception *from inside* a false-if-exception? how can that happen?

do i miss something, or is this a guile bug?

if this seems to be a guile bug, then i'll try to set up a simpler reproducer than my current one. in that case, what may be the key difference between the repl and the shepherd test suite? simply compiled vs. evaluated code?


a hypothesis:
-------------

i tried to look around guile's codebase, and STRING->NUMBER seems to be an optimized or otherwise specially treated primitive.

maybe that special treatment interferes with exceptions? maybe it throws a kind of exception that false-if-exception doesn't catch? or in a way that doesn't get caught?

this call of STRING->NUMBER is within the bootstrapped part of guile itself, which again may add an extra layer complexity or special teatment?

note that if i unset the env var then STRING->NUMBER is called with #f instead of an empty string. maybe there's a bug in how STRING->NUMBER handles being called with a non-string?

i didn't notice anything obviously wrong around SCM_VALIDATE_STRING.

any hint is appreciated,

-- 
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Watch your thoughts; they become words. Watch your words; they become actions. Watch your actions; they become habit. Watch your habits; they become character. Watch your character; it becomes your destiny.”
	— Lao Tzu (sixth century BC)

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

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 5793 bytes --]

[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.)


[-- Attachment #2: Type: text/html, Size: 18473 bytes --]

Re: exception from inside false-if-exception?

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Attila Lendvai <attila@lendvai.name> writes:

> dear fellow Guilers,
>
> context:
> --------
>
> i'm working on shepherd (with several non-trivial local commits). its
> test suite runs clean from a shell, but fails when i try to
> `./pre-inst-env guix build -K shepherd@0.10.99-git`.
>
>
> the sympthom:
> -------------
>
> COLUMNS is not set in the guix build env, and the basic.sh test fails with the following exception/backtrace in the test log:
>
> [...]
> In ice-9/boot-9.scm:
>   1747:15  8 (with-exception-handler [...])
> In shepherd/support.scm:
>     613:9  7 (_ . _)
> In unknown file:
>            6 (display-backtrace [...])
> In system/repl/debug.scm:
>    148:36  5 (print-frames [...])
> In ice-9/boot-9.scm:
>    2137:6  4 (_)
>   1747:15  3 (with-exception-handler [...])
> In system/repl/debug.scm:
>     72:40  2 (_)
> In ice-9/boot-9.scm:
>   1685:16  1 (raise-exception _ #:continuable? _)
>   1683:16  0 (raise-exception _ #:continuable? _)
> ice-9/boot-9.scm:1683:16: In procedure raise-exception:
> In procedure string->number: Wrong type argument in position 1 (expecting string): #f
>
> the expression pointed to by debug.scm,72:40 is this:
>
> (false-if-exception (string->number (getenv "COLUMNS")))

I think I've had similar problems in the past, I did fine this IRC
conversation:

  https://logs.guix.gnu.org/guile/2021-01-19.log#204926

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: exception from inside false-if-exception?

[Attila Lendvai]

> I think I've had similar problems in the past, I did fine this IRC
> conversation:
> 
> https://logs.guix.gnu.org/guile/2021-01-19.log#204926

the very same bug was already discussed in 2021?!

from the log:

> you're using a pre-unwind handler here
> whereas false-if-exception only sets a post-unwind handler
> so yours gets to run first

oh my, what a mess!

do i get this right? (with-exception-handler ... #:unwind? #f) installs a so called pre-unwind-handler, which takes priority over a false-if-exception, even if deeper in the stack, because f-i-e installs a post-unwind-handler?

if this is the case, then i'll basically need to review all the scheme code i wrote in the past two years... :)

i came from common lisp, and from a distance i always had this impression of scheme that it is a much more cleaned up lisp... but in CL the condition system is way cleaner/simpler than this. i wrote long running, multi-threaded web services with advanced logging (backtraces, user installable backtrace decorators, etc), error handlers that deal with nested errors, etc... so i'm rather familiar with the problem domain, but apparently i'm lost in the forest here.

maybe the native call/cc primitive brings in a kind of complexity that is not present in CL around exceptions, unwinding, and backtraces.

> All uses of false-if-exception are wrong

i second this sentiment! our CL team had a syntax for IGNORE-ERRORS, its CL equivalent, to be colored bright red in emacs. and it doesn't even have the pre/post-unwind-hook baggage.

thank you both for the quick feedback!

-- 
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“A society that puts equality — in the sense of equality of outcome — ahead of freedom will end up with neither equality nor freedom. The use of force to achieve equality will destroy freedom, and the force, introduced for good purposes, will end up in the hands of people who use it to promote their own interests.”
	— Milton Friedman and Rose Friedman

RE: exception from inside false-if-exception?

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 3584 bytes --]

>do i get this right? (with-exception-handler ... #:unwind? #f) installs a so called pre-unwind-handler, which takes priority over a false-if-exception, even if deeper in the stack, because f-i-e installs a post-unwind-handler?

It’s not post-unwind? If anything, I would call it pre-unwind. If with-exception-handler were post all the unwinding, then it’s too late to let raise-continuable return anything. 

But other than that (“[it] takes priority over a false-if-exception, [...]), that’s my understanding of things.

I guess what should be done, is it _not_ having priority, and instead it only handling things when there noguard/catch/... inside has caught the exception.

>i came from common lisp, and from a distance i always had this impression of scheme that it is a much more cleaned up lisp... but in CL the condition system is way cleaner/simpler than this. i wrote long running, multi-threaded web services with advanced logging (backtraces, user installable backtrace decorators, etc), error handlers that deal with nested errors, etc... so i'm rather familiar with the problem domain, but apparently i'm lost in the forest here.

First, I want to say that (purely semantically) conditions have nothing to do with exceptions in Guile – you can, technically, raise anything as an exception, not only conditions, and conditions are just a kind of record type that are conventionally used for exceptions (and convenient for that!) but could in principle be used for entirely different things.  So from now on I’ll talk about Guile _exceptions_ instead.

Going by < 9.1 Condition System Concepts | Common Lisp (New) Language Reference (lisp-docs.github.io)>, the API and semantics of CL is pretty much the same as guard/with-exception-handler/raise(-continuable) stuff(*).

(*) Actually, ‘guard’ as documented _always_ handles the exception, it just re-raises them. It’s not mentioned in Guile, but according to RnRS, it’s re-raised in the raise-continuable sense. So this way, it (in theory) has the same semantics as CL stuff.

Looking a bit further, there is also this ‘restart’ stuff in CL. But it seems Scheme already has this in the form or ‘guard’ + ‘raise-continuable’ (if the exception returns a value X, then the (raise-continuable [...]) evaluates to X). 

I don’t think it’s completely equivalent, but it seems to provide similar functionality, and with some effort you could implement something closer to CL with delimited continuations (+ maybe parameters, or their slightly more general API, fluids).

There is ‘warn’ which at first seems not to have an equivalent in Guile, but all that would need to be done is a top-level exception handler

[define a &ignorable-by-default condition]
(define (warn) (raise-continuable [&message stuff + &warning + &ignorable-by-default]))
(guard (c ((ignorable-by-default? c) [print stuff about c] (values)))
  [run main.scm or whatever]).

(I haven’t found search results on what backtrace decorators are in CL.)

>maybe the native call/cc primitive brings in a kind of complexity that is not present in CL around exceptions, unwinding, and backtraces.

There are some implications w.r.t. not misusing parameters for the exception handler stack (IIRC, Guile uses the more general fluid stuff instead because of this), but otherwise, I don’t think so. I think the problem is just a bug (and some of the with-throw-handler stuff that’s making thinking about what’s correct/implementing stuff correctly harder).

Best regards,
Maxime Devos

[-- Attachment #2: Type: text/html, Size: 6904 bytes --]