Make sure we fully clean up state when closing a peasant#11217
Make sure we fully clean up state when closing a peasant#112172 commits merged intomicrosoft:mainfrom
Conversation
ghost
added
Area-Remoting
…stead of crashes.
|
@zadjii-msft I added a test for this particular case. I confirmed that the test runs forever (or at least until I killed it) without the code change, and with the change it passes. It is identical to In the future it might be worth evaluating in the tests if they should use |
|
Being top contributor to this project is so easy
|
you've basically described my entire time on the Console team |
|
@msftbot merge this in 4 minutes |
|
Hello @DHowett! I think you told me that you want to delay the approval for a certain amount of time, but I am not confident that I have understood you correctly. Please try rephrasing your instruction to me. |
|
@msftbot merge this in 1 minute |
ghost
added
the
AutoMerge
|
Hello @DHowett! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
zadjii-msft
left a comment
zadjii-msft
left a comment
There was a problem hiding this comment.
Thanks for adding tests here. We may (in general) just need more tests now that a peasant can close gracefully rather than just "no longer exist", but this will probably cover the biggest case. Thanks!
ghost
merged commit
|
🎉 Handy links: |
ghost
mentioned this pull request
3 participants
Fix infinite loop when trying to summon a window after close.
In #10972 code was added to try to clean up state manually when a window
was closed instead of waiting for it to be detected as a dead peasant.
Unfortunately I didn't know any better and missed cleaning up
_mruPeasantsas well. The result is there would be an infinite loopin
_getMostRecentPeasantsince_getPeasantwill only clean up ids ifit finds a peasant, not if it doesn't find anything. This is the minimal
change to get this working, but it might be a good idea to make
_getPeasantbe more thorough about cleanup.Validation Steps Performed
Tested that before the change we infinitely loop, and after the change
we summon correctly.
Closes #11215