fix(repl): prevent panic when deleting globalThis.closed property #24014
Conversation
|
|
| let closed = self | ||
| .evaluate_expression("(this.closed)") | ||
| .evaluate_expression(&expression) |
There was a problem hiding this comment.
@dsherret it's still set by calling close().
There was a problem hiding this comment.
Won't that only set on globalThis?
There was a problem hiding this comment.
It would, but if you delete it then calling close() will just throw an error saying that it's not defined. Which is okay. But since we check value of globalThis.closed in the REPL look it causes a panic when deleting globalThis.
There was a problem hiding this comment.
Yeah, but this code always returns false now because it's only set on startup. It seems the repl::close_command test is failing now because this is never set to true.
There was a problem hiding this comment.
I thought this.closed is always evaluated on every access since it is a getter, but from the test I can see that it is only evaluated once. I think I will change this line "closed: this.closed" to getter so it should be evaluated every access. But when the closed property is deleted should I set a default value to return or what??
There was a problem hiding this comment.
I guess another option that we can try is to save the reference to globalThis in the "internals" object and then access the getter from this saved reference.
There was a problem hiding this comment.
@bartlomieju Using the getter won't prevent the panic issue. Alternatively, should I return a default value if this.closed is returned undefined?
safaa-mojahed
force-pushed
the
fix-panic-delete-closed-property
branch
from
c388c5a
to
53b8dee
Compare
…adding a reference in the REPL_INTERNALS_NAME object and return default value in case closed property is deleted.
This PR fixes #23422.
When the user deletes the globalThis.closed property, a panic occurs due to the closing function iteratively reading this.closed value in the REPL. This pull request prevents such panics by adding a reference for the closed property in the get_prelude() function for the REPL_INTERNALS_NAME object. The closing function now reads this property from the REPL_INTERNALS_NAME object instead of directly from this.closed.