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

Add 2 parameters to Start-ThreadJob: timeout seconds, priority #18999

Closed
kasini3000 opened this issue Jan 21, 2023 · 15 comments
Closed

Add 2 parameters to Start-ThreadJob: timeout seconds, priority #18999

kasini3000 opened this issue Jan 21, 2023 · 15 comments
Labels
Issue-Enhancement the issue is more of a feature request than a bug Resolution-External The issue is caused by external component(s).

Comments

@kasini3000
Copy link

Summary of the new feature / enhancement

Add 2 parameters to Start-ThreadJob:
timeout seconds
priority

Proposed technical implementation details (optional)

A single asynchronous task has no timeout and no priority, It is very original.

The priority range is 1-9, and the default is 5,priority 1 will be executed first.

@kasini3000 kasini3000 added Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. labels Jan 21, 2023
@rhubarb-geek-nz
Copy link

Can you explain what you expect the timeout to do? What happens to code that is executing in the ThreadJob when the timeout expires?

@rhubarb-geek-nz
Copy link

rhubarb-geek-nz commented Jan 25, 2023

I suggest any timeout parameters should be consistent with Start-Sleep and priority consistent with ThreadPriority

@kasini3000
Copy link
Author

kasini3000 commented Jan 26, 2023

The timeout parameter is passed in when creating a thread. This should be the basic function of the basic library.

Python has function, golang has. C #, powershell, does not have this function

Have a way to function, on github?
What powershell module, or who

@rhubarb-geek-nz
Copy link

What is the effect the timeout supposed to have? As you say C# and PowerShell do not have this capability and PowerShell is built using C#. Neither does the underlying Win32 threading API.
The POSIX pthread library does have the concept of cancelling threads, but for it to work, the code has to safely support cancellation, and it includes API such as pthread_setcancelstate to indicate when it is safe or not for the cancellation to occur, and you have to use pthread_cleanup_push and pthread_cleanup_pop to add handlers to clean up your resources in the case of a cancellation. The Win32 API does not have an equivalent of this for threads, and the Win32 API TerminateThread is strongly discouraged. So the question is what is the timeout supposed to do? Wait-Job already has a timeout so you don't have to wait for a job to complete if it takes too long, but terminating arbitrary code in the middle of execution is not a good idea. An alternative is for a condition on the thread to be set when the timeout has elapsed and then throw an exception. The question is when is this check made and how can it be done in a safe manner so that code cleans up correctly. The existing mechanisms for code to cleanup are based on try and finally so this would seem that the safest way to exit a thread is by throwing an exception within the thread so it will correctly unroll the stack. But if this is based on exceptions, any code that catches that exception will prevent the cancellation. In order for this to be done safely in the pthread environments, there are a list of blocking APIs that may cause the cancellation stack unwinding, in the pthread world this unwinding cannot be prevented with a catch, only by using pthread_setcancelstate to prevent the unwinding starting. The C# asynchronous programming model using Task/await/async does have a cancellation mechanism but this quite a different programming style to threading.

@rhubarb-geek-nz
Copy link

rhubarb-geek-nz commented Jan 26, 2023

In theory you could use QueueUserAPC to inject a callback procedure which calls RaiseException, this would then cause structured exception handlers to unwind, but what actually happens is a complete trashing of the internal state of libraries which do not expect threads to suddenly start unwinding at unexpected points. Been there - done that. You then try replacing the RaiseException with a longjmp, this gives the same result as longjmp on Win32 also uses SEH.

@kasini3000
Copy link
Author

How dare you ask golang to cancel "context. WithTimeout"?
to ask python discard timeout* in threading lib
I want to say that the background thread without timeout is primitive and stupid.

Why does C # and PowerShell not provide background thread-timeout?
For memory leaks caused by bad code, HANG, I think thread-timeout is a good medicine.

@daxian-dbw
Copy link
Member

daxian-dbw commented Jan 27, 2023

Please open the issue in https://github.com/PowerShell/ThreadJob instead.

GitHub
Contribute to PowerShell/ThreadJob development by creating an account on GitHub.

@daxian-dbw daxian-dbw added Resolution-External The issue is caused by external component(s). and removed Needs-Triage The issue is new and needs to be triaged by a work group. labels Jan 27, 2023
@rhubarb-geek-nz
Copy link

Unfortunately the Thread.Abort does not work

Unhandled exception. System.PlatformNotSupportedException: Thread abort is not supported on this platform.
   at System.Threading.Thread.Abort()

See also On replacing Thread.Abort() in .NET 6, .NET 5 and .NET Core

@rhubarb-geek-nz
Copy link

@kasini3000 I think what you are looking for is CancellationTokenSource.CancelAfter Method
The closest equivalent mechanism in dotnet is the CancellationToken, however it is cooperative, voluntary and not global.
The mechanism is that any operation that may take time has to check the cancellation token to see if it is still allowed to run. Looking through the PowerShell source shows this is used in many tools, but if ThreadJob is an external module and has a cancellation token, I don't see a mechanism to force all the other code running in that thread to check with that token.
So this won't be any use for stopping code that is not periodically checking a specific instance of the token.

@mklement0
Copy link
Contributor

@daxian-dbw, in general I wonder if it's better to report ThreadJob issues here, given that:

  • the module now ships with PowerShell
  • its functionality is closely tied to ForEach-Object -Parallel, which is part of this repo.

This would simplify matters and help discoverability.

@mklement0
Copy link
Contributor

mklement0 commented Jan 28, 2023

@rhubarb-geek-nz, while all the technical background you provide and the questions you raise are interesting, note that we already do have thread (runspace) termination:

  • via Stop-Job when applied to Start-ThreadJob-created jobs
  • and there's even a -TimeoutSeconds parameter on the thread-based ForEach-Object -Parallel

I don't know what mechanism is used to terminate threads / runspaces, but clearly the functionality is already available and raising concerns about how it works and whether variant functionality is called for would be a separate debate.

  • At least hypothetically, adding -TimeoutSeconds to Start-ThreadJob could use the same mechanism.
  • As for surfacing a timeout: Given that the job model is a polling-based one, a property on a thread-job object could indicate whether a timeout occurred, such as a specific exception reported in .JobStateInfo.Reason, alongside .State and .JobStateInfo.State reporting Failed, or perhaps introducing a new JobState enum value.

@rhubarb-geek-nz
Copy link

That is good news. My expectation would exposing the actual CancellationToken so that it can be used directly by cmdlets, eg StartSleepCommand.cs should sleep by waiting on the cancellation token rather than the ManualResetEvent. I notice that the WebCmdlet uses CancellationTokens, if this is same one used to abort the thread then cancelling a thread can very efficient.

@rhubarb-geek-nz
Copy link

rhubarb-geek-nz commented Jan 29, 2023

For memory leaks caused by bad code, HANG

So to be clear, a CancellationToken based thread termination is cooperative and not preemptive and requires the target thread to still be responsive.

@kasini3000
Copy link
Author

i open new issue on threadjob:
PowerShell/ThreadJob#26

add PerTimeoutSecond to “ForEach-Object -Parallel”
#18027

@ghost
Copy link

ghost commented Jan 30, 2023

This issue has been marked as external and has not had any activity for 1 day. It has been be closed for housekeeping purposes.

@ghost ghost closed this as completed Jan 30, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement the issue is more of a feature request than a bug Resolution-External The issue is caused by external component(s).
Projects
None yet
Development

No branches or pull requests

4 participants