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?

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. Apr 7, 2011 at 13:38
  • Hey - where's that comment gone? :-) 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. 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. Apr 7, 2011 at 13:50

3 Answers 3

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;
4
  • 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. Apr 7, 2011 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 7, 2011 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. Apr 7, 2011 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. Apr 7, 2011 at 15:00
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

1
  • 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. :-) Apr 7, 2011 at 14:10
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;
8
  • 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;? Apr 7, 2011 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. Apr 7, 2011 at 14:17
  • @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. ;-) Apr 7, 2011 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 :) ? Apr 7, 2011 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. Apr 7, 2011 at 14:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.