0

How would you "extract" nested try/finally blocks from a routine into a reusable entity? Say I have

procedure DoSomething; var Resource1: TSomeKindOfHandleOrReference1; Resource2: TSomeKindOfHandleOrReference2; Resource3: TSomeKindOfHandleOrReference3; begin AcquireResource1; try AcquireResource2; try AcquireResource3; try // Use the resources finally ReleaseResource3; end; finally ReleaseResource2; end; finally ReleaseResource1; end; end;

and want something like

TDoSomething = record // or class strict private Resource1: TSomeKindOfHandleOrReference1; Resource2: TSomeKindOfHandleOrReference2; Resource3: TSomeKindOfHandleOrReference3; public procedure Init; // or constructor procedure Done; // or destructor procedure UseResources; end; procedure DoSomething; var Context: TDoSomething; begin Context.Init; try Context.UseResources; finally Context.Done; end; end;

I want this to have the same exception-safety as the nested original. Is it enough to zero-initialize the ResourceN variables in TDoSomething.Init and do some if Assigned(ResourceN) then checks in TDoSomething.Done?

Improve this question
4
  • @Mr. Disappointment: If it eases your pain imagine the three nested blocks are extracted into three routines which are then called in a nested way. :-) Doesn't change the core of the issue. Commented Apr 7, 2011 at 13:38
  • Hey - where's that comment gone? :-) Commented Apr 7, 2011 at 13:38
  • It does - I removed the comment as I got over the 'OMG!' moment rather speedily as I'm not a Delphi guy, and suddenly recalled a lot worse. ;) But, I would query: why can't you simply use a single try / finally, determining which resource wasn't acquired, and disposing of the ones that were? I guess that's where you're headed with your approach though. Commented Apr 7, 2011 at 13:43
  • Yes. I just asked because I don't want to throw the exception-safety out of the window inadvertently. Commented Apr 7, 2011 at 13:50

3 Answers 3

Reset to default
5

There are three things about classes that make this idiom safe and easy:

  1. During the memory-allocation phase of the constructor (before the real constructor body runs), class-reference fields get initialized to nil.
  2. When an exception occurs in a constructor, the destructor is called automatically.
  3. It's always safe to call Free on a null reference, so you never need to check Assigned first.

Since the destructor can rely on all fields to have known values, it can safely call Free on everything, regardless of how far the constructor got before crashing. Each field will either hold a valid object reference or it will be nil, and either way, it's safe to free it.

constructor TDoSomething.Create; begin Resource1 := AcquireResource1; Resource2 := AcquireResource2; Resource3 := AcquireResource3; end; destructor TDoSomething.Destroy; begin Resource1.Free; Resource2.Free; Resource3.Free; end;

Use it the same way you use any other class:

Context := TDoSomething.Create; try Context.UseResources; finally Context.Free; end;
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

I can't take advantage of item 3 because ReleaseResource isn't necessarily a call to TObject.Free. But I can fix this with if Assigned and item 2 seems to be the important point anyway. So I'll stay with the idiomatic way and reap the benefits.
I usually put the contructor call inside the try..finally block. I just realized (per your point 2) that it is not necessary. I am unsure if I will change my habit, though, as I find it more clear. What do you think?
Follow the example of TObject.Free and FreeMem and make ReleaseResource safe to call on a null resource. It makes things so much easier everywhere else.
Not only is it not necessary, @PA, it's wrong. If the constructor throws, then the variable you're assigning the result to is not initialized, so you're calling Free on an uninitialized value.
1

Yes, you can use a single try/finally/end block for multiple resources with zero-initialization.

Another possible solution can be found in Barry Kelly blog

Improve this answer

1 Comment

Thanks for that idea. Some kind of guard interface already crossed my mind but it always seems overkill to me. BTW: I commented on Barry's post. :-)
1

The pattern with testing on Assigned in finally is used in the Delphi source. You do kind of the same thing but I think you should move Context.Init to capture exception from Context.Init.

procedure DoSomething; var Context: TDoSomething; begin try Context.Init; Context.UseResources; finally Context.Done; end; end;

Edit 1 This is how you should do it without Context.Init and Context.Done. If you place all AquireResource code before try you will not free Resource1 if you get an exception in AcquireResource2

procedure DoSomething; var Resource1: TSomeKindOfHandleOrReference1; Resource2: TSomeKindOfHandleOrReference2; Resource3: TSomeKindOfHandleOrReference3; begin Resource1 := nil; Resource2 := nil; Resource3 := nil; try AcquireResource1; AcquireResource2; AcquireResource3; //Use the resources finally if assigned(Resource1) then ReleaseResource1; if assigned(Resource2) then ReleaseResource2; if assigned(Resource3) then ReleaseResource3; end; end;
Improve this answer

8 Comments

Hmm, putting the Init inside the try block seems wrong. Do you propose that because I made TDoSomething a record type. If it were a class would you write try Context := TDoSomething.Create;?
It seems wrong because it is wrong. If initialization throws an exception, you don't want to finalize anything because you won't know what's safe to finalize.
@Mikael: That's probably the same issue as in stackoverflow.com/q/398137/35162. I remember there were a lot discussions about very subtle points. ;-)
@Ulrich - Yes, the accepted answer is the same as I suggests. Does this mean that your question should be marked as duplicate :) ?
No, Mikael, the accepted answer there is not the same as what you've shown here. If AcquireResource2 throws an exception, Resource2 and Resource3 will not be initialized yet, so it is an error to check their current values with Assigned. You need to initialize things before entering a try block.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.