Easy Timeout #2539
Replies: 72 comments
-
Why do you need new syntax for this? You can already do this with Tasks and CancellationTokens: using (var cts = new CancellationTokenSource())
{
cts.CancelAfter(10000);
try
{
var result = await SomethingAsync(cts.Token);
// Logic for success
}
catch (OperationCanceledException)
{
// Logic for timeout
}
} Of course, this requires the method itself to take in a CancellationToken, which is not always the case. For those cases, you'll want to do what David Fowler recommends here. |
Beta Was this translation helpful? Give feedback.
-
As I said:
Here are few problems associated with the solution you offered above:
|
Beta Was this translation helpful? Give feedback.
-
What do you mean by this @NoamWhy? In both the examples, the timeout login has been separated; in yours, it's been even more separated! I can't see how your mechanism accesses local variables or why this is a positive. Additionally, would this mechanism absorb Exceptions?
Try-catch would still probably be implemented under the hood, like how a using statement implements one. |
Beta Was this translation helpful? Give feedback.
-
Consider this example: void SomeFunction ()
{
var URLs = new List<string>();
// Build URLs list...
var Results = new List<string>();
timeout(10000)
{
foreach (var URL in URLs)
{
Results.Add(GetDataFromURL(URL));
}
}
else
{
// Some alternative logic ....
}
// Process results ....
}
Then this would not be the optimal implementation, if it hurts performance. |
Beta Was this translation helpful? Give feedback.
-
Tough luck, because the CLR doesn't expose aborting a function outside of throwing. |
Beta Was this translation helpful? Give feedback.
-
Also regarding this: using (var cts = new CancellationTokenSource())
{
cts.CancelAfter(10000);
try
{
var result = await SomethingAsync(cts.Token);
// Logic for success
}
catch (OperationCanceledException)
{
// Logic for timeout
}
} What if you want to pass some other cancellation token you got to the SomethingAsync function? |
Beta Was this translation helpful? Give feedback.
-
@NoamWhy Then you can use |
Beta Was this translation helpful? Give feedback.
-
What's the total number of try/catch statements used in David Fowler's solution (including the ones hidden in all the using statements) ? I have counted at least 3 I can see. |
Beta Was this translation helpful? Give feedback.
-
Try-catch is not as slow as you think. Even so, the three try-catches in the solution would be needed implicitly anyway by the mechanism you propose. |
Beta Was this translation helpful? Give feedback.
-
To elaborate, I do like the idea - I still don't understand CancellationTokens in a good amount of detail! But I think the focus should be on more abstraction than performance. |
Beta Was this translation helpful? Give feedback.
-
My guess is that it is something to do with timeouts and that under some circumstances, the else-block is presumably run. Did I guess right? Or to put it another way, asking us to guess what your proposal is, isn't very constructive. I literally am forced to guess what the code is supposed to do. So actually detailing the proposal, and explaining why additional syntax would be better than implementing it with methods, would be far more constructive. |
Beta Was this translation helpful? Give feedback.
-
You guessed correctly! And this is precisely the point I was trying to make, i.e. this syntax is so simple and clean, that you can hardly misinterpret it. If you are still not sure why "additional syntax would be better than implementing it with methods", take a second look at the current alternative below, and see if it's as easy to guess what it's doing: void SomeFunc()
{
try
{
Task.Run (() =>
{
//Do some timed logic
}).TimeoutAfter(new TimeSpan(0,0,1000)).Wait();
}
catch (OperationCanceledException)
{
// Logic for timeout
}
}
public static async Task<T> TimeoutAfter<T>(this Task<T> task, TimeSpan timeout)
{
using (var cts = new CancellationTokenSource())
{
var delayTask = Task.Delay(timeout, cts.Token);
var resultTask = await Task.WhenAny(task, delayTask);
if (resultTask == delayTask)
{
// Operation cancelled
throw new OperationCanceledException();
}
else
{
// Cancel the timer task so that it does not fire
cts.Cancel();
}
return await task;
}
} |
Beta Was this translation helpful? Give feedback.
-
It's simple and clean to you because you created the idea, but there are a whole lot of questions that came to mind when I read this post ...
As @DavidArno wrote:
Every language features starts out at -100 points. For any proposal to get traction, the proposal needs to clearly articulate not only why it's valuable, but why it is dramatically superior to what's already possible. Another way to achieve a very similar result, would be to use a helper method like this:
How is this so bad? |
Beta Was this translation helpful? Give feedback.
-
While I agree with many of your points, the "magic 100000 in what units" is
on dodgy ground as the language already has that concept in other places.
…On Sat, 18 May 2019, 11:06 Bevan Arps, ***@***.***> wrote:
And this is precisely the point I was trying to make, i.e. this syntax is
so simple and clean, that you can hardly misinterpret it.
It's simple and clean *to you* because you created the idea, but there
are a whole lot of questions that came to mind when I read this post ...
- What's the magic 10000 figure?
- Is it some kind of flag? If so, what does it mean?
- If it's a duration, what are the units? Processor Ticks? Seconds?
Jiffys?
- What does the *else* clause do?
- Normally an *else* separates two mutually exclusive execution paths
- how is the decision made on whether to run the first branch or the second?
As @DavidArno <https://github.com/DavidArno> wrote:
asking us to guess what your proposal is, isn't very constructive.
Every language features starts out at -100 points
<https://blogs.msdn.microsoft.com/ericgu/2004/01/12/minus-100-points/>.
For any proposal to get traction, the proposal needs to clearly articulate
not only why it's valuable, but why it is dramatically superior to what's
already possible.
Another way to achieve a very similar result, would be to use a helper
method like this:
void SomeFunction ()
{
// Some initial logic ....
RunWithTimeout(
10.Seconds(),
async () => {
// Some more logic
});
// Some final logic ....
}
How is this so bad?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<https://github.com/dotnet/csharplang/issues/2539?email_source=notifications&email_token=ADIEDQOVEFWI7ZIKZW2FLKTPV7ILNA5CNFSM4HNZQW3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVWLV4A#issuecomment-493665008>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADIEDQNQOOCSSOXIWBALU2LPV7ILNANCNFSM4HNZQW3A>
.
|
Beta Was this translation helpful? Give feedback.
-
Here is a better syntax, if you will: void SomeFunction ()
{
// Some initial logic ....
for (10000 milliseconds) try
{
// Some more logic ....
}
catch(TimeoutException)
{
// Some alternative logic ....
}
// Some final logic ....
} |
Beta Was this translation helpful? Give feedback.
-
That won't actually work. You need to await Task.WhenAny. The 'Task' you get back from it isn't TimerTask (or the Task from Task.Run) it's a new task that when awaited will give you one of those tasks back. I would highly recommend trying to make a working example of how the translated code would even look like. :) |
Beta Was this translation helpful? Give feedback.
-
Not true, since I am using Task.WaitAny and not Task.WhenAny. void SomeFunction ()
{
var QuitFun = true;
if(0 == Task.WaitAny
(
Task.Delay(1000).Start();
,Task.Run
( () =>
{
// Do some timed logic
QuitFun = false;
}
)
)
) throw new TimeoutException();
else if (QuitFun) return;
} |
Beta Was this translation helpful? Give feedback.
-
The fact that we're reaching for Tasks or any threading mechanism I think is reason enough that this can't be a simple language feature. There are just way too many gotchas here that a simple language feature would hide away from the developer. For example, the fact that the calling thread would block is a very common vector for deadlocks, and why mixing async/sync is generally considered a Bad Idea. |
Beta Was this translation helpful? Give feedback.
-
That's precisely the whole purpose of high level languages.
I don't see the problem, can you describe such a scenario?
No such mix is suggested. For sync functions we will use WaitAny, and for async we will use WhenAny and await. |
Beta Was this translation helpful? Give feedback.
-
If you really want it, why not propose it as a simple all in one function in dotnet/corefx rather than as a language feature? |
Beta Was this translation helpful? Give feedback.
-
It's not the purpose of high level languages to take everything that can be done in helper functions and turn it into language features.
Which is blocking (sync) an async operation (the Task). One example is if the calling method holds a Another example is UI interaction. Of the calling method is on the UI thread then the code within |
Beta Was this translation helpful? Give feedback.
-
Fair enough, so programmers should be aware of all these issues, but this is not an excuse to abandon altogether any attempt to simplify timeout implementation. |
Beta Was this translation helpful? Give feedback.
-
Yes, so make a method to do it for you, and suggest it is added to the standard library. Why is a language feature needed? |
Beta Was this translation helpful? Give feedback.
-
Why do we need foreach (...) if we can use for (...) and while(Enum.MoveNext())? The answer to all these question is - efficiency, clarity, and ease. |
Beta Was this translation helpful? Give feedback.
-
It is a big reason for not trying to embed it into the language with syntax that belies all of the concerns you need to be deeply aware of in order to use it.
None of them have the problems that this feature has. |
Beta Was this translation helpful? Give feedback.
-
Simply inform programmers that the timed code is executed in a separate thread/task, and should be treated like any other such code. Done! |
Beta Was this translation helpful? Give feedback.
-
People use them significantly more than a timeout feature. And seemingly everyone except you here agrees on that |
Beta Was this translation helpful? Give feedback.
-
As I wrote above:
|
Beta Was this translation helpful? Give feedback.
-
Unfortunately if it's not common, it's very unlikely to be implemented - the C# team have a limited amount of time to work on things with broad application like NRTs. |
Beta Was this translation helpful? Give feedback.
-
Timeouts in general are much better suited to a programming library rather than the language itself. Remember, C# doesn't specify the threading model or whatever for a lot of code - especially async code - that's done entirely in .NET, using types like TaskScheduler and ThreadPool. Adding support for timeouts directly in the language would mean C# would have to start specifying threading models, concurrency approaches, etc. and that means that it would take a lot of time to even think about getting right. And then what happens if for a specific person they get those options wrong? They'd be fighting against the language to get their specific task done, and likely making lots of mistakes along the way. To me, this honestly feels like a Pit Of Despair solution to a problem that can easily be solved in library code. For example, you could have a method: Task<TResult> CancelAfter<TResult>(TimeSpan timeout, Func<CancellationToken, Task<TResult>> callback) { throw null; } And this one method would look, honestly, just as nice as your syntax: async Task MyMethodAsync()
{
await CancelAfter(TimeSpan.FromSeconds(5),
async (cancelToken) =>
{
// code here
cancelToken.ThrowIfCancellationRequested();
});
} |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Try to guess what the following suggested syntax is designed for:
With this kind of syntax, implementing timeout functionality becomes trivial.
Sure, timeout can be implemented today using various combinations of currently available multi-threading library functions, but none of these implementations is remotely as simple and clean as the code above.
P.S.
As a result of the discussion below, I am suggesting here the following better syntax:
Beta Was this translation helpful? Give feedback.
All reactions