Another exception on close, maybe not a crash? #11684
Labels
Area-Remoting
Communication layer between windows. Often for windowing behavior, quake mode, etc.
Help Wanted
We encourage anyone to jump in on these.
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Priority-1
A description (P1)
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
zadjii-msft
added
Help Wanted
ghost
added
the
Needs-Triage
|
There is a |
|
Yup, that makes sense and is definitely something I overlooked or didn't notice because of catching the exception, and the fact that I made it so calling I don't really have time to work on anything here for the next couple of weeks, but it can be solved in a number of ways
Probably should also give some thought to the placement of SignalClose itself. Originally it was just called when the WindowManager ran its destructor, but of course in the linked PR I added it earlier in the closing process and so some of the assumptions have been broken. |
DHowett
removed
the
Needs-Triage
6 tasks
ghost
pushed a commit
that referenced
this issue
Dec 6, 2021
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request Clean up an invalid access that I introduced in #11440 <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes #11684 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
ghost
added
the
Resolution-Fix-Committed
DHowett
pushed a commit
that referenced
this issue
Dec 13, 2021
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request Clean up an invalid access that I introduced in #11440 <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes #11684 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed (cherry picked from commit 29e6235)
|
🎉This issue was addressed in #11857, which has now been successfully released as Handy links: |
DHowett
removed
the
zPreview-Service-Queued-1.13
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This one seems to hit 100% for me in Debug builds, currently on 1167e20 but I'd bet this is in
main.Repro:
Likely regressed in #11440, this line: https://github.com/microsoft/terminal/pull/11440/files#diff-d9af234d6dd21db9240dc3733d69471898a79f1c4386cb3c48ac45ab7158cd07R405
AppHost::LastTabClosedRESETS the_getWindowLayoutThrottler, then callsSignalClose(). That goes up and back down to the_windowManager.WindowClosedhandler set up inAppHost::_BecomeMonarch. At this point,_getWindowLayoutThrottlerDOESN'T HAVE A VALUE, socauses a
std::bad_optional_accessexception.I don't know if this is a crash, or just a first-chance exception that VS is reporting that doesn't matter. It's definitely a bug though.
Stack
The text was updated successfully, but these errors were encountered: