Issue #9725 has been updated by Charles Nutter.


Yukihiro Matsumoto wrote:
> I am interested why referencing the target object could cause problem. The target object must be existed before the exception, and exception should disappear soon after handling.

I'm not sure it's the referencing that's really the problem here, it's that NameError#inspect will inspect the object, possibly causing side effects. The case I've seen in ActiveRecord is when you have a lazy set or tree of results that won't load until walked...but then inspect fires and has to walk the whole thing. You end up having a NameError bubble out somewhere, get inspected, and trigger all this loading that will at least be overhead and at worst cause a different error, wiping out the first.

Why does NameError need to inspect the target object at all? If it just carried it along, users could opt-in to get more info on the target by calling NameError#target.Nobuyoshi Nakada wrote:

> Building the message at creation time does not reduce memory usage and CPU time at all, just makes it earlier.

Agreed.

> And `#inspect` method of such complex class should consider it, I don't think that huge output is not human-readable.

Perhaps. Should #inspect never trigger lazy loads? Should inspect not recurse down large hierarchies? The problem here is that you could get a NameError against almost anything, like a giant array with a bunch of giant arrays in it.

> Anyway, unexpectedly huge result of inspection often causes problems, so the uncaught exception handler shows the class and the object ID only in that case, but it needs to build the inspection string once and discards then.
> It's a long-time known issue that we need smarter way for inspection.

That's all well and good, but this is a real problem now, and I don't think having NameError inspect the failed target adds as much as it risks.

I also suggested that the inspect behavior could remain, but perhaps only if we're in verbose mode. In that case, getting the fully-inspected NameError is just a flag away.

----------------------------------------
Feature #9725: Do not inspect NameError target object unless verbose
https://bugs.ruby-lang.org/issues/9725#change-52340

* Author: Charles Nutter
* Status: Open
* Priority: Normal
* Assignee: 
----------------------------------------
At least once every few months, we get an error report of JRuby raising a memory error where MRI does not due to `NameError`'s `Message` object holding a reference to an object that's too large to inspect. I propose that this inspection of the target object should only be done in verbose mode.

## Background:

`NameError` is raised when a variable-like method call fails to find a defined method. The resulting exception is created with a hidden `NameError::Message` that holds the object in which the method could not be found.

When name error needs to render its message, such as when it bubbles out or when `#message` is called, it does `to_str` on the `NameError::Message`, which ends up inspecting the target object. If this object's inspect output is large (or infinite) it can end up consuming a large amount of memory.

## Problems:

* If the amount of memory required to render a `NameError` exceeds available memory, a very confusing and misleading memory error can be raised instead.
* If the target object is considered sensitive data, it will end up bubbling out through potentially untrustworthy code. It is an encapsulation flaw, basically.
* A `NameError` that gets held in memory will also prevent GC of the object it references.

## Solutions:

* `NameError` should not capture the target object.
* `NameError` should build a message based on the target object *at creation time*, and only include information useful to indicate the type of object.
* (Optional) If verbose mode is set, `NameError` can just do what it does now.




-- 
https://bugs.ruby-lang.org/