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

AIX: Breakpoint resolved isn't handled properly on restart #35

Closed
jkrems opened this issue Mar 15, 2017 · 8 comments
Closed

AIX: Breakpoint resolved isn't handled properly on restart #35

jkrems opened this issue Mar 15, 2017 · 8 comments

Comments

@jkrems
Copy link
Collaborator

jkrems commented Mar 15, 2017

See TODO in test/cli/preserve-breaks.test.js.

Expected output of breakpoints after restart:

#0 examples/three-lines.js:2
#0 examples/three-lines.js:3

Actual output (still showing the regex for matching the filename that should've been resolved):

#0 .*//path/to/node-inspect/examples/three-lines.js$:2
#1 .*//path/to/node-inspect/examples/three-lines.js$:3

These "fake filenames" are generated when setting a breakpoint in a file that has been loaded yet:

// inspect_repl.js
        if (!bp.location) { // Fake it for now.
          Object.assign(bp, {
            actualLocation: {
              scriptUrl: `.*/${script}$`,
              lineNumber: line - 1,
            },
          });
@gibfahn
Copy link
Member

gibfahn commented Mar 17, 2017

cc/ @CurryKitten

@mhdawson
Copy link
Member

@CurryKitten can you take a look at this so we can discussion when we talk on Friday ?

@CurryKitten
Copy link

@mhdawson Ok - I'm looking into it

@CurryKitten
Copy link

I'm finding that I can no longer recreate this issue. I could previous recreate is very easily just by running node-inspect against test/three-lines.js, setting multiple breakpoints and then restarting a few times.

I've been running it through several hundred iterations, can't recreate the failure. We still have the warning that the file hasn't been loaded yet (as well as on other platforms) but the regex in the path is properly resolved. Did the delay added in #34 also fix this ?

@jkrems
Copy link
Collaborator Author

jkrems commented Apr 20, 2017

Maybe..? Should we remove the AIX special case in the test and see how it goes?

Trott added a commit to Trott/io.js that referenced this issue Jul 2, 2021
Remove code that made a check more lenient to account for a known issue
that is no longer reproducible.

Refs: nodejs/node-inspect#35
@Trott
Copy link
Member

Trott commented Jul 2, 2021

Optimistically closing based on nodejs/node#39238

@Trott Trott closed this as completed Jul 2, 2021
@Trott
Copy link
Member

Trott commented Jul 2, 2021

Hmmm....although I guess it should stay open until https://github.com/nodejs/node/blob/1544e69b93565c97e7ed232bf2db237d34b8fd7c/test/common/debugger.js#L26-L29 is resolved/removed.

@Trott Trott reopened this Jul 2, 2021
Trott added a commit to Trott/io.js that referenced this issue Jul 6, 2021
Remove code that made a check more lenient to account for a known issue
that is no longer reproducible.

Refs: nodejs/node-inspect#35

PR-URL: nodejs#39238
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
@Trott
Copy link
Member

Trott commented Jul 9, 2021

Workaround code has been removed so this at least appears to be fixed.

@Trott Trott closed this as completed Jul 9, 2021
targos pushed a commit to nodejs/node that referenced this issue Jul 11, 2021
Remove code that made a check more lenient to account for a known issue
that is no longer reproducible.

Refs: nodejs/node-inspect#35

PR-URL: #39238
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
targos pushed a commit to nodejs/node that referenced this issue Sep 4, 2021
Remove code that made a check more lenient to account for a known issue
that is no longer reproducible.

Refs: nodejs/node-inspect#35

PR-URL: #39238
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants