Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

fix Issue 14036 - Do not throw FinalizeError on OutOfMemoryError or Inva... #1123

Merged
merged 1 commit into from Jan 27, 2015

Conversation

CyberShadow
Copy link
Member

...lidMemoryOperationError

This changes FinalizeError to be thrown only when an Exception is thrown,
with the disadvantage of not catching non-Exception Throwables and the
advantage of preserving the stack trace for the code that caused
InvalidMemoryOperationError.

https://issues.dlang.org/show_bug.cgi?id=14036

@CyberShadow
Copy link
Member Author

CC @MartinNowak


assert( test!Exception);
import core.exception : InvalidMemoryOperationError;
assert(!test!InvalidMemoryOperationError);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you'll have to catch the memory error to successfully pass the unittest.

@rainers
Copy link
Member

rainers commented Jan 25, 2015

I agree that not catching Errors is better.
Slightly related: we should use some special assert inside the GC to avoid an InvalidMemoryError because onAssertError is allocating.

…nvalidMemoryOperationError

This changes FinalizeError to be thrown only when an Exception is thrown,
with the disadvantage of not catching non-Exception Throwables and the
advantage of preserving the stack trace for the code that caused
InvalidMemoryOperationError.
@CyberShadow
Copy link
Member Author

Finally fixed tests.

@MartinNowak
Copy link
Member

Auto-merge toggled on

@MartinNowak
Copy link
Member

Why was the stacktrace of Throwable.next missing?

@CyberShadow
Copy link
Member Author

We can allocate neither a stack trace nor a FinalizeError in an InvalidMemoryOperation condition.

@CyberShadow
Copy link
Member Author

To clarify, I was talking about the stack trace in the debugger. If we catch and rethrow, the stack trace of the point where the exception was thrown is lost (unless it was saved in the exception object, which it was not).

MartinNowak added a commit that referenced this pull request Jan 27, 2015
fix Issue 14036 - Do not throw FinalizeError on OutOfMemoryError or Inva...
@MartinNowak MartinNowak merged commit cfe3eb5 into dlang:master Jan 27, 2015
@Orvid
Copy link
Contributor

Orvid commented Jan 27, 2015

With an InvalidMemoryError or OutOfMemoryError, would it be a viable choice to simply claim that all memory currently allocated is free to be used by the runtime to output useful information, such as a stack trace. Both of these errors are impossible to recover from, so the best we can do is hope that the current state is at least valid enough to be able to use the space for a stack trace.

@CyberShadow
Copy link
Member Author

That might do more harm than good, e.g. corrupt user data.

@CyberShadow
Copy link
Member Author

It would also be easier to use C malloc, or allocate directly from the OS (for the case of InvalidMemoryOperation).

@Orvid
Copy link
Contributor

Orvid commented Jan 27, 2015

A short transcript of the ensuing IRC conversation:

CyberShadow: In both of those cases though, there's nothing you can do about it, the program IS going to crash, so why does it matter if we corrupt user data?
The user might be dumb and set up some atexit() handlers OSLT even though they shouldn't
But for it to corrupt their data, they'd have to be using GC allocated memory; As we are already known to be crashing, the entirety of the application's memory should be assumed to be corrupted, we are merely being wishful that the GC's core tables haven't been.

@MartinNowak
Copy link
Member

A stack trace for OOM is misleading and InvalidMemoryError will be fixed at some point.
We could probably still use malloc for the stacktrace.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants