Extract nested try/finally blocks

Question

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?

@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. — Uli Gerhardt, Apr 07 '11 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. — Grant Thomas, Apr 07 '11 at 13:43
Yes. I just asked because I don't want to throw the exception-safety out of the window inadvertently. — Uli Gerhardt, Apr 07 '11 at 13:50

score 5 · Accepted Answer · answered Apr 07 '11 at 14:30

5

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

During the memory-allocation phase of the constructor (before the real constructor body runs), class-reference fields get initialized to nil.
When an exception occurs in a constructor, the destructor is called automatically.
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;

answered Apr 07 '11 at 14:30

Rob Kennedy

161,384
21
275
467

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. – Uli Gerhardt Apr 07 '11 at 14:38
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? – PA. Apr 07 '11 at 14:55
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. – Rob Kennedy Apr 07 '11 at 14:59
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. – Rob Kennedy Apr 07 '11 at 15:00

score 1 · Answer 2 · answered Apr 07 '11 at 13:58

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

answered Apr 07 '11 at 13:58

kludg

27,213
5
67
118

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. :-) – Uli Gerhardt Apr 07 '11 at 14:10

Mikael Eriksson · Answer 3 · 2011-04-07T15:01:15.047

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;

edited Apr 07 '11 at 15:01

answered Apr 07 '11 at 14:00

Mikael Eriksson

136,425
22
210
281

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;`? – Uli Gerhardt Apr 07 '11 at 14:13
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. – Rob Kennedy Apr 07 '11 at 14:17
@Mikael: That's probably the same issue as in http://stackoverflow.com/q/398137/35162. I remember there were a lot discussions about **very** subtle points. ;-) – Uli Gerhardt Apr 07 '11 at 14:44
@Ulrich - Yes, the accepted answer is the same as I suggests. Does this mean that your question should be marked as duplicate :) ? – Mikael Eriksson Apr 07 '11 at 14:47
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. – Rob Kennedy Apr 07 '11 at 14:57
@Rob - I will update with the necessary initialization code. From the question I assumed that was being done in Init. "Is it enough to zero-initialize the ResourceN variables in TDoSomething.Init" – Mikael Eriksson Apr 07 '11 at 14:59
@Mikael: I must admit that my question looks quite like a duplicate in retrospective. – Uli Gerhardt Apr 07 '11 at 15:01
@Mikael: I accepted Rob's answer because it looks more idiomatic/Delphi'ish and is probably what the language designers *want* me to write. Thank you for your input! – Uli Gerhardt Apr 07 '11 at 15:11

Extract nested try/finally blocks

3 Answers3

Linked