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

[Bug] 连续ctrl+c ,窗口卡死 #2789

Closed
4 tasks done
siruoren opened this issue Dec 1, 2022 · 11 comments
Closed
4 tasks done

[Bug] 连续ctrl+c ,窗口卡死 #2789

siruoren opened this issue Dec 1, 2022 · 11 comments

Comments

@siruoren
Copy link

siruoren commented Dec 1, 2022

Version Information

Cmder version: 1.3.20
Operating system: win0 专业版

Cmder Edition

Cmder Full (with Git)

Description of the issue

[Bug] 连续ctrl+c ,窗口卡死

How to reproduce

连续ctrl+c

Additional context

No response

Checklist

  • I have read the documentation.
  • I have searched for similar issues and found none that describe my issue.
  • I have reproduced the issue on the latest version of Cmder.
  • I am certain my issues are not related to ConEmu, Clink, or other third-party tools that Cmder uses.
@DRSDavidSoft
Copy link
Contributor

对不起,不清楚你在说什么。Post an screenshot of the problem if possible.

@siruoren
Copy link
Author

siruoren commented Dec 5, 2022

对不起,不清楚你在说什么。Post an screenshot of the problem if possible.

image
连续ctrl+c,然后就会卡在当前状态

@DRSDavidSoft
Copy link
Contributor

What is your Surface device? Arm or x64 Intel?

@siruoren
Copy link
Author

siruoren commented Dec 5, 2022

What is your Surface device? Arm or x64 Intel?

x64 Intel

@DRSDavidSoft
Copy link
Contributor

Thanks, please write in English, and can you capture a video, I still can not understand what is happening.
You can write more descriptive messages in Chinese if you prefer.
thanks.

@chrisant996
Copy link
Contributor

@DRSDavidSoft what @siruoren is reporting is that holding down Ctrl-C eventually causes a hang in the cmd shell.

I am able to reproduce that inside Cmder with the 1.3.20 release (which is the current at the moment).

I attached a debugger to the cmd.exe process, and the hang is occurring because the Ctrl-C occurred while the prompt filter was invoking git, and the background git.exe process has become stuck within a git.exe --no-pager config cmder.cmdstatus command. Killing the hidden git.exe allows cmd.exe to regain control.

Some observations:

  • The prompt is still slow to show up each time, even with the async prompt stuff. I'll debug why it's being slow and share the findings. I suspect that calling git.exe --no-pager config cmder.cmdstatus every single time to figure out whether to skip invoking git is probably what's making it slow. If so, I'll add some kind of timeout so it doesn't invoke that unless it's been more than some number of seconds since the prompt was last displayed (maybe 5 or 10 seconds, so that holding Enter or Ctrl-C is fast).
  • I'm not sure whether Clink can do anything about the Ctrl-C interrupting git, and git getting stuck as a result. I'll try to see if I can track down what's going on inside git, and look for a workaround or solution.

@chrisant996
Copy link
Contributor

chrisant996 commented Dec 5, 2022

I suspect that calling git.exe --no-pager config cmder.cmdstatus every single time to figure out whether to skip invoking git is probably what's making it slow.

Confirmed:

The git config calls are not hooked up to use the async prompt filtering, so they cause the prompt to be very slow every single time. Adding a 10 second timeout indeed makes the prompt super fast to show up when holding down Enter or Ctrl-C.

Also, the git config calls are invoked at every prompt, regardless whether the current directory is in a git repo.

So the best solution is to make these changes:

  1. Move the get_git_status_setting() call somewhere inside the if git_dir then scope.
  2. Move the get_git_status_setting() call somewhere inside the get_git_info_table() call.
  3. Add a timeout so that it doesn't invoke them until at least 10 seconds since the last time it invoked them.

I've made those changes and confirmed they produce a significant and noticeable improvement, especially outside of a git directory.

I'll send a pull request after I finish investigating why Ctrl-C is causing git to hang.

@chrisant996
Copy link
Contributor

chrisant996 commented Dec 5, 2022

I'm not sure whether Clink can do anything about the Ctrl-C interrupting git, and git getting stuck as a result. I'll try to see if I can track down what's going on inside git, and look for a workaround or solution.

Fixing the issue about how get_git_status_setting() was being invoked seems to also make the Ctrl-C problem disappear.

With that fix in place, I held Ctrl-C for over a minute, and:

  1. Prompts showed up instantaneously each time.
  2. The shell didn't hang at any point.

But I'll still try to track down specifically why git.exe was hanging.

@chrisant996
Copy link
Contributor

chrisant996 commented Dec 5, 2022

Here's what happens:

  1. The prompt filter invokes io.popen("git --no-pager config cmder.cmdstatus 2>nul") which does not use async prompt filtering, and immediately launches git using the current console.
  2. Internally, that actually launches a new cmd.exe to handle the 2>nul redirection, and the new cmd.exe in turn launches the git command.
  3. Depending on timing, the Ctrl-C may be handled by the new cmd.exe and cause it to exit. But the git.exe it launched is still running.
  4. The original cmd.exe shell is waiting for output from the git.exe (and the new cmd.exe in between them) to finish. But the output never finishes, because git.exe gets stuck.

I tried with several other programs being launched instead of git.exe. But only git.exe gets stuck. (I made the other programs produce varying degrees of output to test whether they get stuck trying to print output after the in-between cmd.exe gets terminated by the Ctrl-C, but they don't get stuck regardless how much output they produce.)

This makes it look like the hang lies inside git.exe itself. Clink and Cmder can't do anything about that, but the changes I made in Cmder's prompt filter successfully avoid the problem in git.exe when async prompt filtering is enabled in Clink (and it is enabled by default).

I'll put together a pull request with the change to Cmder's prompt script.

@DRSDavidSoft
Copy link
Contributor

@chrisant996 Wow, that was fast! Thanks for the investigation and the quick bug fix. I didn't understand what was happening as described in the bug report at first, so this is a huge help.

Quick question, do you think it would be worthwhile to also report the hang to the git-for-windows developers? It's awesome that clink would handle situations where it would hang, but it'd be best if we could prevent it in the first place, since as you said it's isolated to git.exe executable at the moment, and no other programs.

Great job, and thanks! 😄 👍🏻

@chrisant996
Copy link
Contributor

PR is #2791

Quick question, do you think it would be worthwhile to also report the hang to the git-for-windows developers? It's awesome that clink would handle situations where it would hang, but it'd be best if we could prevent it in the first place, since as you said it's isolated to git.exe executable at the moment, and no other programs.

To report the issue to git would require tracking down a minimal repro that does not involve Cmder or Clink or Lua. It's certainly possible to do, but it would take a day or two.

There were 3 bugs in the Cmder prompt script regardless, and once those bugs are fixed then git.exe no longer hangs. If I were on the git team and received a report "git has a bug, and when we fixed our bugs the git bug went away" then I would want to see more research and evidence to substantiate the claim that there's a bug in git. On the surface it's easy for it to sound like the bug must have been in the caller, rather than in git.

And I can't tell if it's git.exe's fault, or if it's the fault of the C runtime implementation from whichever compiler is used to build git.exe for Windows, or if it's an inherent OS issue involving pipes that take over the current console, or something else.

So, I'm not interested in spending time to come up with a minimal repro for the problem occurring in git.exe. It's not worth the couple of days of investment if no one else has encountered similar problems in a console mode .exe program that invoke git repeatedly in rapid succession (and I think such programs are exceedingly rare to begin with).

There isn't enough info to file an actionable issue, and I typically avoid filing an unsubstantiated claim against some code base, because I intensely dislike receiving such claims myself. They're expensive to respond to in a polite and constructive manner, and the first two questions will always be "how can I investigate this without installing 3 other programs and learning how to configure them and use them well enough to reproduce the problem -- and how have you reached the conclusion that the problem isn't in one of the other programs?" 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants