Handle SERIOUS-CONDITIONs during build

Bug #1605650 reported by Robert P. Goldman on 2016-07-22
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Robert P. Goldman

Bug Description

Memory exhaustion failures on ECL signal SERIOUS-CONDITION but not ERROR. Need to check for these during ASDF operations and handle them appropriately (i.e., in the same way we handle ERRORs).

See https://gitlab.com/embeddable-common-lisp/ecl/issues/264 for details.

Robert P. Goldman (rpgoldman) wrote :

OK, this is not an ECL-specific problem, it's just that I never encountered it on other lisps.

summary: - Handle ECL SERIOUS-CONDITIONs during build
+ Handle SERIOUS-CONDITIONs during build
Robert P. Goldman (rpgoldman) wrote :


Here are some thoughts:

1. Change *FATAL-CONDITIONS* in UIOP to be '(SERIOUS-CONDITION) instead of '(ERROR)?
2. We could change the error-handler in FIND-SYSTEM (the one that raises LOAD-SYSTEM-DEFINITION-ERROR) to use SERIOUS-CONDITION instead of ERROR.
3. We could check for SERIOUS-CONDITION in MODULE-PROVIDE-ASDF (operate.lisp)

The thing that bothers me about 2 & 3 is that I believe that this would make errors that should be continuable (simply request more memory and continue if successful) into errors that become fatal and not continuable.

Faré (fahree) wrote :

This probably should be discussed on the asdf-devel mailing-list, maybe also the pro mailing-list and/or requesting advice from various implementers and language lawyers.

Robert P. Goldman (rpgoldman) wrote :

It's clear from the spec that #1 is the Right Thing, as long as I understand your code. Your docs say:

"conditions that cause the Lisp image to enter the debugger if interactive,
or to die if not interactive"

The hyperspec for SERIOUS-CONDITION is almost word-for-word:

"All conditions serious enough to require interactive intervention if not handled should inherit from the type serious-condition."

So I am going to make that change.

For number 2, I think it's more about the structure of our code -- can we allow the user to intervene and handle some of these conditions? Are we inadvertently preventing the user from successfully invoking a restart? I'm not sure I understand well enough how that condition handling is being done in ASDF. E.g., in FIND-SYSTEM, we wrap an original error in a LOAD-SYSTEM-DEFINITION-ERROR. I'm not sure, but I suspect that makes inaccessible any original restarts.

For #3, MODULE-PROVIDE-ASDF, we are just providing the user with additional information -- we don't handle the error, we just tell the user that this error has caused loading the module to fail. So I can change that ERROR to SERIOUS-CONDITION without denying the user access to restarts.

I'll make change #3 also.

Robert P. Goldman (rpgoldman) wrote :

Given that what we are doing in case #2 is just to wrap other errors in instances of load-system-definition-ERROR, and I'm asking what to do about SERIOUS-CONDIITONs that are NOT ERRORs, I'm making the executive decision *not* to modify the code for FIND-SYSTEM.

I'm open to objections, but for now I've made modifications numbers 1 & 3 and am closing this bug after I verify that the modifications pass all our tests.

Robert P. Goldman (rpgoldman) wrote :

My fix #1 causes breakage on CCL. CCL uses a SERIOUS-CONDITION called RESET-PROCESS in its shutdown. For now I have a patch to mask this serious condition from being treated as a FATAL-CONDITION, and that fixes all the tests. I'm waiting to see if the Clozure folks have any suggestions about this before proceeding: I'm not sure why this is a SERIOUS-CONDITION in CCL, since it doesn't seem like something that should be handled interactively if not handled automatically.

Changed in asdf:
assignee: nobody → Robert P. Goldman (rpgoldman)
status: New → In Progress
Robert P. Goldman (rpgoldman) wrote :

Fixed in version

Changed in asdf:
status: In Progress → Fix Committed
Changed in asdf:
milestone: none → 3.3
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers