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

Graceful shutdown does not seems to happen with default profile that launches PowerShell #13582

Closed
thadguidry opened this issue Jul 23, 2022 · 5 comments
Labels
Issue-Question For questions or discussion Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@thadguidry
Copy link

thadguidry commented Jul 23, 2022

Windows Terminal version

1.15.2002.0

Windows build number

10.0.22622.0

Other Software

OpenRefine 3.6

Please work with our developers to help understand this issue more, as we want the best Windows 11 experience for our OpenRefine users.
Our mailing list dev thread for this issue is here: https://groups.google.com/g/openrefine-dev/c/f8krfDFGaQg/m/tj2SX0vRAgAJ

Steps to reproduce

  1. download and double click on the OpenRefine 3.6 executable file openrefine.exe (bottom left screen)
  2. the default profile on Windows 11 launches and runs the application using PowerShell (top left screen)
  3. I press ctrl+c on my keyboard and the process is exited immediately with no graceful shutdown (likely because the signals sent to the application are different now?)
  4. If I open a PowerShell in Terminal tab (top right screenshot) and manually launch OpenRefine with .\openrefine.exe it will run and if I press ctrl-c on my keyboard, then it seems the correct signals are sent and OpenRefine begins it's graceful shutdown as it has done consistently in conhost.exe and cmd.exe since Windows 98.
  5. Modify the default termination behavior with "closeOnExit": "graceful" and save (bottom right screen) and test the default profile again by double-clicking on openrefine.exe
  6. then in the launched new terminal running OpenRefine, press ctrl+c to attempt to gracefully shutdown, but notice that it continues to exhibit an immediate exit and display code (oxc000013a) and seemingly not sending the same signals as it does if the application is launched by running the command line .\openrefine.exe.

Expected Behavior

image

I would expect that since PowerShell is being used in both the default profile in the run app context (double clicking openrefine.exe and the manually launched Terminal in the command line context (.\openrefine.exe), that it would result in the same termination behavior, but it does not.

Actual Behavior

the default profile with PowerShell seems to exhibit slightly different termination signals sent to the application when in the context of a command-line (.\openrefine.exe) or a run behavior (double-clicking on openrefine.exe icon)
We are not confident that the same signals are being sent in both contexts, such as historically $? = 128 + SIGINT = 130

@thadguidry thadguidry added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Jul 23, 2022
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jul 23, 2022
@thadguidry thadguidry changed the title Graceful shutdown does not seems to happen with default profile that launchs PowerShell Graceful shutdown does not seems to happen with default profile that launches PowerShell Jul 23, 2022
@DHowett
Copy link
Member

DHowett commented Jul 25, 2022

Interesting! So, there's something strange afoot here.

Look at this:

image

In orange, the PowerShell prompt comes back. In green, however, the application continues producing output.

If I had to guess, it's that OpenRefine spawns a subprocess that is attached to the same console as the original/root OpenRefine process. Further, that root process is terminating with the ^C error code.

Now, Terminal tracks the root process and terminates the connection when that process exits.

When there's a shell, Terminal ends up keeping the connection open until the shell exits. When OpenRefine is running directly inside Terminal, and it gets a ^C, the connection stops since the root process exits.

This is an area in which we're a bit deficient: we don't track an entire process tree the way the console used to, so we are susceptible to a little bit of a behavioral difference in process lifetime and teardown order.

There's a couple ways to fix this. One is going to be to wait for us to fix it -- we have some plans here -- which might take a while. The other is for OpenRefine to effectively join/wait-for its child process to terminate after getting the signal before exiting the parent.

Thoughts?

@zadjii-msft
Copy link
Member

As an aside - when you run openrefine.exe, the profile that gets used isn't the profile set as the defaultProfile, it instead uses the settings from the profiles.defaults block. So putting closeOnExit: graceful into the powershell.exe profile won't apply to when you run openrefine.exe directly. That's something that changed fairly recently, I wanna say around 1.12.

@zadjii-msft zadjii-msft added Issue-Question For questions or discussion Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Issue-Bug It either shouldn't be doing this or needs an investigation. labels Jul 25, 2022
@thadguidry
Copy link
Author

thadguidry commented Jul 25, 2022

So, the suggestion to fix on our end or your end is...? What is the timeline fix on your end...about a year or less? Then that might be fine for us actually.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 25, 2022
@zadjii-msft
Copy link
Member

Sorry, this must've gotten lost in the queue.

There's a couple ways to fix this. One is going to be to wait for us to fix it -- we have some plans here -- which might take a while.

I don't know for sure what Dustin was talking about here - he's being a little too coy for me to decipher this time. He might have meant the closeOnExit: automatic thing(#13560), which I believe shipped in the latest 1.15 release.

Does that work more as expected/?

If not, then I'm not really sure about the other idea he was talking about.

@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Dec 5, 2022
@thadguidry
Copy link
Author

Tested closeOnExit: automatic and it did not perform a graceful shutdown. We will look into improving the situation on our end.
Closing for now.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Question For questions or discussion Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Development

No branches or pull requests

3 participants