Jump to content
Der schöne Günther

ITask.Wait() behaving differently when called multiple times

Recommended Posts

Posted (edited)

Consider the following:

 

uses System.Threading;

procedure TForm1.Button1Click(Sender: TObject);
var
	task: ITask;
begin
	task := TTask.Run(
		procedure()
		begin
			raise EProgrammerNotFound.Create('test');
		end
	);

	try
		task.Wait(); // << raises EAggregateException
	except
		ShowMessage('ex');
	end;

	task.Wait(); // no exception. returns true
	// task.Status is now TTaskStatus.Exception
end;

I find this irritating. Why does waiting for a task raise an exception one time, but not the other?

Apart from the fact that the Embarcadero documentation on ITask.Wait(..) does not even mention an exception should be expected, I guess they (once again) modelled it after .NET's Task.Wait(..) method which clearly states an exception is to be expected if the task was cancelled or failed with an exception.

 

Some fine gentleman even reported this to Embarcadero almost five years back:

https://quality.embarcadero.com/browse/RSP-9548

 

But two years later, it was closed as "Works as expected".

 

I still don't understand. Can someone elaborate? I believe the implementation is wrong.

Edited by Der schöne Günther
confused EOperationCancelled with EAggregateException

Share this post


Link to post
Posted (edited)

In 10.3 it actually raises an EAggregateException with the EProgrammerNotFound as inner exception.

I guess because the exception mechanism in Delphi destroys handled exceptions they cannot be raised that easily a second time as .NET does

 

@Dmitry Arefiev Documentation should be expanded a little though.

Edited by Stefan Glienke

Share this post


Link to post

Does that mean 10.3 raises an exception for both times?

 

Because I accidentally typed it would raise an EOperationCancelled. It does, in fact, properly raise an EAggregateException. But just for the first time.

Share this post


Link to post
Posted (edited)

The exception is stored in the Task object and once it is raised it will be removed and it can not be raised again because of lifetime mangement (raise will free the exception)

Edited by Schokohase

Share this post


Link to post
56 minutes ago, Der schöne Günther said:

Does that mean 10.3 raises an exception for both times?

No, because

1 minute ago, Schokohase said:

The exception is stored in the Task object and once it is raised it will be removed and it can not be raised again because of lifetime mangement (raise will free the exception)

although technically not raise will free but the code running after any exception handling code unless you called AcquireExceptionObject.

Share this post


Link to post
Posted (edited)

But "the exception" is an artificially generated EAggregateException that references exceptions that are already stored. The "true" exceptions have never been raised directly.

 

There would be nothing wrong with raising an EAggregateException that does not own the exceptions it referred and instead the task can free those exceptions when it is destroyed. Which is what was suggested in https://quality.embarcadero.com/browse/RSP-9548.

 

Wouldn't that solve all those problems?

Edited by Der schöne Günther

Share this post


Link to post

Hello raise

 

please free any exception after the exception handling, but not when the exception is the one that is stored inside the task object.

 

But what should happen when someone aqcuires the exception. In normal case you have taken over the resposibility for the lifetime management of that exception. Maybe another flag: No not this exception it is managed elsewhere?

Share this post


Link to post

There is no need to change anything about raising exceptions or lifetime management of exceptions.

 

It's just that an EAggregateException always owns the exception it refers to. I believe that should be optional.

 

Raise the exception normally, or acquire ownership and free it later - It doesn't matter. As long as the task owns its exceptions, not the EAggregateException. You can do with the AggregateExcption whatever you like.

 

It's not the same, but TThread is similar. It has a FatalException property you can query for the exception that happened in the thread. The exception is owned by the thread. It is there for as long as the thread object is alive. With the way tasks are implemented, the first one to call .Wait() on a task gets the exceptions - After that, they're gone. It makes no sense.

Share this post


Link to post
Posted (edited)

Yes, but the thread exception will not be raised except you do it by your code. That is different to ITask where it will be raised if you wait for the task.

 

 

Edited by Schokohase

Share this post


Link to post

And because of that, it's even more important the exceptions do not get lost and you get a reliable and reproducible behaviour. 😎

 

As so often, it seems to be modelled after .NET. And what .NET does (raise the exception(s) every  time you wait for the task) is perfectly achievable with Delphi. I just don't understand why they took this weird approach.

Share this post


Link to post
Posted (edited)
4 hours ago, Der schöne Günther said:

And what .NET does (raise the exception(s) every  time you wait for the task) is perfectly achievable with Delphi.

And how would you propose to do that exactly?

 

Think of the use-case where the ITask is freed before the EAggregateException is caught.  The RTL can't go back after the fact and say "hey, EAggregateException, when I raised you, I gave you some exceptions to hold references to but not take ownership of, but I'm going away now so please take ownership of them now, thanks".  The only way I can think of solving this is by making the inner exceptions be reference counted.  But Embarcadero/Idera is moving away from object-based ARC in RAD Studio 10.4, so such refcounting would have to be implemented manually by ITask and EAggregateException going forward.  And then what happens if the user catches EAggregateException wants to extract the inner exceptions and re-raise them (which you yourself asked about just the other day)?  So now the user has to get involved in the refcounting mechanism, or otherwise make EAggregateException relinquish ownership.

 

This is not a very good scenario.

Edited by Remy Lebeau

Share this post


Link to post

Thank you for replying.

 

If that can happen, then yes, then that won't work. But right now, I have no idea how that could happen, to be honest.

 

In order to catch the exception, I have to call .Wait() on the task - Meaning I have to have a reference to it. Meaning the task cannot be freed yet. What am I missing?

Share this post


Link to post
Posted (edited)
10 hours ago, Der schöne Günther said:

In order to catch the exception, I have to call .Wait() on the task - Meaning I have to have a reference to it. Meaning the task cannot be freed yet. What am I missing?

For instance, think of the Task being created and freed in a try/finally block that is inside of a try/except block.  Or if a try/except block calls a function, which creates and frees the Task before exiting.  In either case, the exception can be caught after the Task is freed.

Edited by Remy Lebeau

Share this post


Link to post

Thank you for taking the time to explain.

 

It sounds like a good point, although a bit artificial 😉

And I have trouble achieving just that.

 

Even with something like

procedure TForm1.Button1Click(Sender: TObject);
var
    task: ITask;
    e: Exception;
begin
    event := TSimpleEvent.Create();
    task := TTask.Run(
        procedure()
        begin
            raise EProgrammerNotFound.Create(EmptyStr);
        end
    );
    try
        try
            task.Wait();
        finally
            task := nil;
        end;
    except
        e := Exception( AcquireExceptionObject() );
    end;
end;

The task still lives. Maybe because the default thread pool still has a reference to it.

Share this post


Link to post
5 hours ago, Der schöne Günther said:

The task still lives

 

Tasks can stay idle for something like 5 minutes and then longer in the retired state.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×