Skip to content

Process TPC Support & Kill Retry as Admin #1320

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

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

watchingfun
Copy link

Plugin allows searching by listening tcp port
After the kill fails, try to execute it again with administrator privileges
#1317

Copy link
Member

@taooceros taooceros left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this feature. There're some style issues that worth taking a look.

I am not pretty familiar with WMI so I may take a look on that later.

Comment on lines 58 to 60
string commandArgument = string.Format("/c netstat -an -o -p tcp|findstr \":{0}.*LISTENING\"", port);

string commandOut = ExecuteCommandAndCaptureOutput(COMMAND_EXE, commandArgument);
Copy link
Member

Choose a reason for hiding this comment

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

I have a feeling that netstat is a exe that can be called directly without cmd. Could you take a try?
Sorry I didn't see the findstr before. Though, I still think we shall be able to process the string directly with either regex or a manually written parser.

}

// split host:port
var hostPortTokens = stringTokens[1].Split(new char[] { ':' });
Copy link
Member

Choose a reason for hiding this comment

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

It's possible to have multiple port in one line? Or that won't happen for Listening port?

process.StartInfo.RedirectStandardError = true;
process.Start();

commandOut = process.StandardOutput.ReadToEnd();
Copy link
Member

Choose a reason for hiding this comment

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

This will be stuck if the process never ends. I think we should put it lower after waiting process ends. Also, probably make this part async.

Copy link
Author

@watchingfun watchingfun Aug 10, 2022

Choose a reason for hiding this comment

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

First of all, thank you very much for reviewing and pointing out the problems 😊, but in addition to some simple style problems, there are many problems that I can't fix due to my limited ability. You should close this pr, if you have time, can you add these two features🥺?sorry for wasting your precious time. (Actually, I know this code is shit, but I don't have the energy to learn how to write better implementations recently. 😢

Copy link
Member

@taooceros taooceros Aug 10, 2022

Choose a reason for hiding this comment

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

I don't think the code is that bad. Those things I mention can be adjusted fairly easily.
Though, if you don't have time, I can work on this based on your work for sure (but I probably will not be free to do that for a while).
Thanks for the enhancement idea and an implementation draft!

Copy link
Author

Choose a reason for hiding this comment

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

Thank you very much, then I close this pr?

Copy link
Member

Choose a reason for hiding this comment

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

Just leave it here. I will finish it once I got time.

Comment on lines +107 to +108
startInfo.FileName = "powershell.exe";
startInfo.Arguments = $"Start cmd.exe -ArgumentList \"/k\",\"taskkill\",\"/f\",\"/pid\", \"{p.Id}\" -Verb Runas";
Copy link
Member

Choose a reason for hiding this comment

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

I forget this one. It's pretty weird that you start taskkill inside a cmd inside the powershell.

Copy link
Member

@taooceros taooceros Aug 10, 2022

Choose a reason for hiding this comment

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

The startInfo do provide an ArgumentList and Verb API that worth taking a look.

@taooceros taooceros marked this pull request as draft August 10, 2022 15:47
@Jack251970 Jack251970 changed the title Enhance: Flow.Launcher.Plugin.ProcessKiller Process TPC Support & Kill Retry as Admin Mar 15, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors Count
❌ forbidden-pattern 22
⚠️ ignored-expect-variant 1
⚠️ non-alpha-in-dictionary 19

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

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

Successfully merging this pull request may close these issues.

3 participants