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

console: do not clear output when a session ends #10671

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

vince-fugnitto
Copy link
Member

What it does

Fixes: #10653.

The pull-request fixes an issue which caused the debug console output to clear when a session was completed.
The change now preserves the output so end-users can view the output of their debug session.

It namely reverts a change proposed in #10333.

How to test

  1. start a debug session (with and without debug breakpoints).
  2. when the session is terminated, the console output should be preserved.
  3. confirm that subsequent sessions also work correctly.

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto vince-fugnitto added the debug issues that related to debug functionality label Jan 26, 2022
@vince-fugnitto vince-fugnitto self-assigned this Jan 26, 2022
@vince-fugnitto
Copy link
Member Author

@msujew do you mind reviewing whenever you get a chance? I reverted the logic from #10333 which broke the output, and am unsure if no longer setting the cached session to undefined will cause any side-effects.

@colin-grant-work
Copy link
Contributor

I believe that the motivation behind removing the reference to the debug session was to ensure that it would be garbage-collected when the session was terminated. With this change, that doesn't happen:

image

The ConsoleWidget isn't the only retainer of old sessions: of the four in that screenshot, two are retained by something in debug-hover-source.ts, one by something in debug-stack-frame.tsx, and it appears that the reference retained by the console is released when the session is replaced by another console session, but it seems that there ought to be some way to release the debug session without deleting the content.

@colin-grant-work
Copy link
Contributor

But if push comes to shove, I think retaining a reference to the session is better than deleting the output, so if there's no way to preserve the output without retaining the reference for now, that's OK.

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Better than the alternative for now, though it may be worth making an issue to allow for retention of output when session is set to undefined.

The commit fixes an issue which caused the debug console output to clear
when a session was completed. The change now preserves the output so
end-users can view the output of their debug session.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Looks good to me as well. I agree with Colin, we can fix the dangling debug session reference sometime else.

@vince-fugnitto
Copy link
Member Author

I'll merge for the moment since it improves the behavior for end-users, and open a follow-up issue to track the cleanup.
Thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug issues that related to debug functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

debug: console is cleared after a debug session
3 participants