Handle SERIOUS-CONDITIONs during build

Bug #1605650 reported by Robert P. Goldman
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fix Released
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.

Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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  
Everyone can see this information.

Other bug subscribers