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

processhelp.pm: improve taskkill calls (Windows) #14959

Closed
wants to merge 4 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Sep 18, 2024

  • drop tasklist call before taskkill.
    taskkill offers two ways to kill a pid:

    1. -pid <pid>
      If <pid> is missing it returns 128 and outputs:
      ERROR: The process "<pid>" not found.
      
    2. -fi "PID eq <pid>"
      If <pid> is missing, it returns 0 and outputs:
      INFO: No tasks running with the specified criteria.
      

    The curl runner script doesn't check the result of the call and both
    stdout and stderr are redirected to NUL.
    Meaning the tasklist calls pre-verifying if the PID exists are not
    necessary and we can drop them to put less strain on the runner
    environment.

  • log a taskkill call missed earlier.
    Follow-up to e53523f CI: move Azure jobs to GHA, fix fallouts, sshserver, runtests tweaks #14859

  • streamline taskkill calls by using the -pid option
    (was -fi <filter-expression>).

  • make taskkill in pidterm() use -t to kill the process tree.

Ref: #11009


w/o whitespace: https://github.com/curl/curl/pull/14959/files?w=1

taskkill has two ways of killing a task:

1. `-pid <pid>`
   If `<pid>` is missing it returns 128 and outputs:
   ```
   ERROR: The process "<pid>" not found.
   ```

2. `-fi "PID eq <pid>"`
   If `<pid>` is missing, it returns 0 and outputs:
   ```
   INFO: No tasks running with the specified criteria.
   ```

The curl runner script doesn't check the result of the call and
both stdout and stderr are redirected to NUL.
@mback2k
Copy link
Member

mback2k commented Sep 18, 2024

Sounds like a good plan, but haven't checked the code changes!

@vszakats vszakats changed the title processhelp.pm: improve taskkill calls processhelp.pm: improve taskkill calls (Windows) Sep 18, 2024
@vszakats vszakats closed this in c997f3e Sep 19, 2024
@vszakats vszakats deleted the w-taskkill branch September 19, 2024 10:44
moritzbuhl pushed a commit to moritzbuhl/curl that referenced this pull request Sep 20, 2024
- drop `tasklist` call before `taskkill`.
  `taskkill` offers two ways to kill a `pid`:
  1. `-pid <pid>`
     If `<pid>` is missing it returns 128 and outputs:
     ```
     ERROR: The process "<pid>" not found.
     ```
  2. `-fi "PID eq <pid>"`
     If `<pid>` is missing, it returns 0 and outputs:
     ```
     INFO: No tasks running with the specified criteria.
     ```
  The curl runner script doesn't check the result of the call and both
  stdout and stderr are redirected to NUL.
  Meaning the `tasklist` calls pre-verifying if the PID exists are not
  necessary and we can drop them to put less strain on the runner
  environment.

- log a `taskkill` call missed earlier.
  Follow-up to e53523f curl#14859

- streamline `taskkill` calls by using the `-pid` option
  (was `-fi <filter-expression>`).

- make `taskkill` in `pidterm()` use `-t` to kill the process tree.

Ref: curl#11009
Closes curl#14959
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants