#110, On Windows ignore ERROR_ACCESS_DENIED for TerminateProcess() if… #111
Conversation
|
Libuv doesn't swallow the error, it returns |
|
The change that i propose only ignores |
|
Ah that's true. I missed the !, still this is not solving your original
problem. It's just hiding it.
…On Thu, Nov 16, 2017, 21:04 Tilman Blumhagen ***@***.***> wrote:
The change that i propose only ignores ERROR_ACCESS_DENIED of
TerminateProcess() if the process did die (and did not exit with
STILL_ACTIVE). In this case we can be sure the process already has
terminated. So this change only makes terminateProcess succeed also when
the process is already gone (even if TerminateProcess() fails to succeed
for that reason).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#111 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABH3KQfE--Ve32Ck_PD5ZF0zeLFK0E3Tks5s3KNegaJpZM4Qg-_1>
.
|
|
It solves it... except for the situation where the subprocess exits with 259 ( I think we have to accept that there is a chain of obstacles ( |
|
How does it solve it. Your process wasn't dead. It was just blocked.
…On Thu, Nov 16, 2017, 21:57 Tilman Blumhagen ***@***.***> wrote:
It solves it... except for the situation where the subprocess exits with
259 (STILL_ACTIVE) where the ERROR_ACCESS_DENIED is propagated just like
before. So for example my test case from #110
<#110> now runs without errors
and correctly terminates its subprocesses. See it more as an improvement
than a complete solution.
I think we have to accept that there is a chain of obstacles (
TerminateProcess() sometimes failing for mysterious undocumented reasons,
GetExitCodeProcess() being helpful to work around that... except for one
specific exit code) and have to make the best out of it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#111 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABH3KT2m9C6h-GL9N5nno4Hxgl9Pt0j8ks5s3K_MgaJpZM4Qg-_1>
.
|
|
What makes you think that? The code path that i added only succeeds if the process actually terminated. So you can be sure the process was dead when |
|
Oh I see, I misinterpreted your example, the hGetChar is in the caller not the child. fair enough. |
|
@snoyberg This is waiting for quite a while now. Could you merge it? |
|
@Mistuke Are you OK with me merging and including this in the next release? |
|
@snoyberg yes no objections from me. Thanks. |
|
@sternmull Just one last request then: would you be able to add a short description of the change into the changelog? Thanks! |
…ss() if the process did terminate To my knowledge this behavior is not officially documented. But it seems multiple people discovered ERROR_ACCESS_DENIED when trying to terminate a process that did already die on its own. For example libuv has a workaround for this.
|
@snoyberg sure. Rebased the PR on master and updated changelog. |
It seems this can be attributed to strange behavior of the Windows API. libuv has a workaround for this: https://github.com/libuv/libuv/blob/master/src/win/process.c#L1172
It is not perfect (because it does work only for processes that do not exit with 259). But to my current knowledge this is a platform specific limitation we have to accept.