Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array<RegExp>.toString() doesn't work properly on Node.js CLI #44417

Closed
yukha-dw opened this issue Aug 27, 2022 · 7 comments · Fixed by #45375
Closed

Array<RegExp>.toString() doesn't work properly on Node.js CLI #44417

yukha-dw opened this issue Aug 27, 2022 · 7 comments · Fixed by #45375
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem. v8 engine Issues and PRs related to the V8 dependency.

Comments

@yukha-dw
Copy link

yukha-dw commented Aug 27, 2022

Version

v16.13.1

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

No response

What steps will reproduce the bug?

Hello, I've found interesting bug on Node.js CLI on Windows Console and also on WSL.

These code will work as intended when being run inside a file (node index.js) or copy to CLI simultaneously, but won't work if we run it line by line.

const arr = [/asd/,/cvb/];

first=`${arr.toString()}`;
console.log({first});

arr.toString();

second=`${arr.toString()}`;
console.log({second});

After running arr.toString(); without assigning to a variable, the arr variable will broke and failed to return correct string.

Output

Scenario: node index.js

{ first: '/asd/,/cvb/' }
{ second: '/asd/,/cvb/' }

Scenario: Copy to Node.js CLI simultaneously

Welcome to Node.js v16.13.1.
Type ".help" for more information.
> const arr = [/asd/,/cvb/];
undefined
>
> first=`${arr.toString()}`;
'/asd/,/cvb/'
> console.log({first});
{ first: '/asd/,/cvb/' }
undefined
>
> arr.toString();
'/asd/,/cvb/'
>
> second=`${arr.toString()}`;
'/asd/,/cvb/'
> console.log({second});
{ second: '/asd/,/cvb/' }
undefined
>

Bugged Scenario: Run Line by Line on Node.js CLI

Welcome to Node.js v16.13.1.
Type ".help" for more information.
> const arr = [/asd/,/cvb/];
undefined
> first=`${arr.toString()}`;
'/asd/,/cvb/'
> console.log({first});
{ first: '/asd/,/cvb/' }
undefined
>
> arr.toString();
''
>
> second=`${arr.toString()}`;
''
> console.log({second});
{ second: '' }
undefined
>

How often does it reproduce? Is there a required condition?

On right condition, it happens all the time.

What is the expected behavior?

arr variable does not change internally and able to return correct string using .toString() method.

What do you see instead?

arr.toString() returns blank string and becomes broken.

Additional information

No response

@cola119 cola119 added the repl Issues and PRs related to the REPL subsystem. label Aug 27, 2022
@cola119
Copy link
Member

cola119 commented Aug 27, 2022

Thank you for reporting! I can reproduce this issue on v18.8.0.

@cola119 cola119 added the confirmed-bug Issues with confirmed bugs. label Aug 27, 2022
@cola119
Copy link
Member

cola119 commented Aug 27, 2022

A minimum code to reproduce

$ node -p "const a = [/a/]; a.toString();"
/a/

$ node
> const a = [/a/]; a.toString();
'/a/'
> a.toString();
''

@vorbrodt
Copy link

Do you think this issue could be suitable for a new contributor? If so, I would like to be assigned to it and give it a try.

@cola119
Copy link
Member

cola119 commented Sep 1, 2022

From my brief investigation, this is seemingly caused by V8 issue, not Node.js. (Node.js just calls V8's APIs to execute scripts line by line, and ChromeDevTools has the same issue too.) I don't know this issue is suitable for you but it is definitely worthwhile to look into the code to find out the cause!

@F3n67u
Copy link
Member

F3n67u commented Sep 4, 2022

A minimum code to reproduce

$ node -p "const a = [/a/]; a.toString();"
/a/

$ node
> const a = [/a/]; a.toString();
'/a/'
> a.toString();
''

A more minimum code to reproduce:

$ node
Welcome to Node.js v18.2.0.
Type ".help" for more information.
> const a = [/a/];
undefined
> a.toString()
''

@BridgeAR BridgeAR added the v8 engine Issues and PRs related to the V8 dependency. label Sep 4, 2022
@BridgeAR
Copy link
Member

BridgeAR commented Sep 4, 2022

I am able to reproduce this with the Chrome DevTools.

I just reported it to the @nodejs/v8 team. https://bugs.chromium.org/p/v8/issues/detail?id=13259

@mmis1000
Copy link

mmis1000 commented Sep 5, 2022

It seems this is not even specific to regex.

PS C:\Users\mmis1> node
Welcome to Node.js v16.13.2.
Type ".help" for more information.
> const b = [{ toString() { console.log(1); return '1' } }]
undefined
> b + ''
''

targos added a commit to targos/node that referenced this issue Nov 8, 2022
Original commit message:

    [runtime] Clear array join stack when throwing uncatchable

    ... exception.

    Array#join depends array_join_stack to avoid infinite loop
    and ensures symmetric pushes/pops through catch blocks to
    correctly maintain the elements in the join stack.
    However, the stack does not pop the elements and leaves in
    an invalid state when throwing the uncatchable termination
    exception. And the invalid join stack state will affect
    subsequent Array#join calls. Because all the terminate
    exception will be handled by Isolate::UnwindAndFindHandler,
    we could clear the array join stack when unwinding the terminate
    exception.

    Bug: v8:13259
    Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451
    Reviewed-by: Jakob Linke <jgruber@chromium.org>
    Commit-Queue: 王澳 <wangao.james@bytedance.com>
    Cr-Commit-Position: refs/heads/main@{#83465}

Refs: v8/v8@031b98b
Closes: nodejs#44417
nodejs-github-bot pushed a commit that referenced this issue Nov 11, 2022
Original commit message:

    [runtime] Clear array join stack when throwing uncatchable

    ... exception.

    Array#join depends array_join_stack to avoid infinite loop
    and ensures symmetric pushes/pops through catch blocks to
    correctly maintain the elements in the join stack.
    However, the stack does not pop the elements and leaves in
    an invalid state when throwing the uncatchable termination
    exception. And the invalid join stack state will affect
    subsequent Array#join calls. Because all the terminate
    exception will be handled by Isolate::UnwindAndFindHandler,
    we could clear the array join stack when unwinding the terminate
    exception.

    Bug: v8:13259
    Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451
    Reviewed-by: Jakob Linke <jgruber@chromium.org>
    Commit-Queue: 王澳 <wangao.james@bytedance.com>
    Cr-Commit-Position: refs/heads/main@{#83465}

Refs: v8/v8@031b98b
Closes: #44417
PR-URL: #45375
Fixes: #44417
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
ruyadorno pushed a commit that referenced this issue Nov 21, 2022
Original commit message:

    [runtime] Clear array join stack when throwing uncatchable

    ... exception.

    Array#join depends array_join_stack to avoid infinite loop
    and ensures symmetric pushes/pops through catch blocks to
    correctly maintain the elements in the join stack.
    However, the stack does not pop the elements and leaves in
    an invalid state when throwing the uncatchable termination
    exception. And the invalid join stack state will affect
    subsequent Array#join calls. Because all the terminate
    exception will be handled by Isolate::UnwindAndFindHandler,
    we could clear the array join stack when unwinding the terminate
    exception.

    Bug: v8:13259
    Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451
    Reviewed-by: Jakob Linke <jgruber@chromium.org>
    Commit-Queue: 王澳 <wangao.james@bytedance.com>
    Cr-Commit-Position: refs/heads/main@{#83465}

Refs: v8/v8@031b98b
Closes: #44417
PR-URL: #45375
Fixes: #44417
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Original commit message:

    [runtime] Clear array join stack when throwing uncatchable

    ... exception.

    Array#join depends array_join_stack to avoid infinite loop
    and ensures symmetric pushes/pops through catch blocks to
    correctly maintain the elements in the join stack.
    However, the stack does not pop the elements and leaves in
    an invalid state when throwing the uncatchable termination
    exception. And the invalid join stack state will affect
    subsequent Array#join calls. Because all the terminate
    exception will be handled by Isolate::UnwindAndFindHandler,
    we could clear the array join stack when unwinding the terminate
    exception.

    Bug: v8:13259
    Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451
    Reviewed-by: Jakob Linke <jgruber@chromium.org>
    Commit-Queue: 王澳 <wangao.james@bytedance.com>
    Cr-Commit-Position: refs/heads/main@{#83465}

Refs: v8/v8@031b98b
Closes: #44417
PR-URL: #45375
Fixes: #44417
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Original commit message:

    [runtime] Clear array join stack when throwing uncatchable

    ... exception.

    Array#join depends array_join_stack to avoid infinite loop
    and ensures symmetric pushes/pops through catch blocks to
    correctly maintain the elements in the join stack.
    However, the stack does not pop the elements and leaves in
    an invalid state when throwing the uncatchable termination
    exception. And the invalid join stack state will affect
    subsequent Array#join calls. Because all the terminate
    exception will be handled by Isolate::UnwindAndFindHandler,
    we could clear the array join stack when unwinding the terminate
    exception.

    Bug: v8:13259
    Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451
    Reviewed-by: Jakob Linke <jgruber@chromium.org>
    Commit-Queue: 王澳 <wangao.james@bytedance.com>
    Cr-Commit-Position: refs/heads/main@{#83465}

Refs: v8/v8@031b98b
Closes: #44417
PR-URL: #45375
Fixes: #44417
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
Original commit message:

    [runtime] Clear array join stack when throwing uncatchable

    ... exception.

    Array#join depends array_join_stack to avoid infinite loop
    and ensures symmetric pushes/pops through catch blocks to
    correctly maintain the elements in the join stack.
    However, the stack does not pop the elements and leaves in
    an invalid state when throwing the uncatchable termination
    exception. And the invalid join stack state will affect
    subsequent Array#join calls. Because all the terminate
    exception will be handled by Isolate::UnwindAndFindHandler,
    we could clear the array join stack when unwinding the terminate
    exception.

    Bug: v8:13259
    Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451
    Reviewed-by: Jakob Linke <jgruber@chromium.org>
    Commit-Queue: 王澳 <wangao.james@bytedance.com>
    Cr-Commit-Position: refs/heads/main@{#83465}

Refs: v8/v8@031b98b
Closes: #44417
PR-URL: #45375
Fixes: #44417
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
Original commit message:

    [runtime] Clear array join stack when throwing uncatchable

    ... exception.

    Array#join depends array_join_stack to avoid infinite loop
    and ensures symmetric pushes/pops through catch blocks to
    correctly maintain the elements in the join stack.
    However, the stack does not pop the elements and leaves in
    an invalid state when throwing the uncatchable termination
    exception. And the invalid join stack state will affect
    subsequent Array#join calls. Because all the terminate
    exception will be handled by Isolate::UnwindAndFindHandler,
    we could clear the array join stack when unwinding the terminate
    exception.

    Bug: v8:13259
    Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451
    Reviewed-by: Jakob Linke <jgruber@chromium.org>
    Commit-Queue: 王澳 <wangao.james@bytedance.com>
    Cr-Commit-Position: refs/heads/main@{#83465}

Refs: v8/v8@031b98b
Closes: #44417
PR-URL: #45375
Fixes: #44417
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants