Implicitly created TTs not deleted when returning with an er

From the help:

• The FINALLY block is an undoable block with implicit ON ERROR UNDO, THROW error handling, and like all blocks, FINALLY blocks implicitly throw all stop objects. You cannot explicitly override the ON ERROR directive for a FINALLY block. If a statement within the FINALLY block raises ERROR, the FINALLY block will be undone, and ERROR will be raised in the block that encloses the associated block of the FINALLY block. Error is not raised in the associated block. Otherwise, infinite looping could occur. The same is true if the FINALLY block raises STOP.

Particularely: 

If a statement within the FINALLY block raises ERROR, the FINALLY block will be undone

So I would say it's expected baviour. IMHO: raising errors in the FINALLY should be avoided at all costs. FINALLY is there for cleanup purposes. You should use a DO ON ERROR UNDO, THROW + CATCH and return the error from the CATCH block.

It makes no difference if you return the error from the FINALLY block or from the procedure itself.

DEFINE TEMP-TABLE ttTest NO-UNDO
  FIELD cValue AS CHARACTER.

DEFINE VARIABLE hTable AS HANDLE NO-UNDO.

ASSIGN hTable = TEMP-TABLE ttTest:HANDLE.
MESSAGE "initial hTable:" hTable hTable:DEFAULT-BUFFER-HANDLE.

RUN doTableLogic(INPUT-OUTPUT TABLE-HANDLE hTable, INPUT TRUE) NO-ERROR.
IF ERROR-STATUS:ERROR
  THEN MESSAGE "doTableLogic error:" RETURN-VALUE.

RUN scanleaks(INPUT SESSION:FIRST-BUFFER).

PROCEDURE doTableLogic:
  DEFINE INPUT-OUTPUT PARAMETER TABLE-HANDLE iphTTBuffer.
  DEFINE INPUT PARAMETER iplReturnError AS LOGICAL     NO-UNDO.

  MESSAGE "iphTTBuffer:" iphTTBuffer.

  IF iplReturnError THEN
    RETURN ERROR "sorry".

  FINALLY:
    /* delete the implicit created TT */
    IF VALID-HANDLE(iphTTBuffer) THEN
      DELETE OBJECT iphTTBuffer.
  END FINALLY.
END.

PROCEDURE scanLeaks:
  DEFINE INPUT  PARAMETER iphHandle AS HANDLE      NO-UNDO.

  IF VALID-HANDLE(iphHandle) THEN
  DO:
    MESSAGE "Handle Leak" iphHandle iphHandle:TABLE-HANDLE iphHandle:NAME.

    /* recurse to find all */
    RUN scanLeaks(INPUT iphHandle:NEXT-SIBLING).
    
    /* clean up */
    DELETE OBJECT iphHandle:TABLE-HANDLE.
  END.
END.

Because you didn't use the do on error, throw.

procedure doTableLogic:

  define input-output parameter TABLE-HANDLE iphTTBuffer.
  define input parameter iplReturnError as logical no-undo.

  do on error undo, throw:

    message "iphTTBuffer:" iphTTBuffer.

  end.
  catch err1 as Progress.Lang.Error :
    if iplReturnError then
      return error "sorry".
  end catch.

  finally:
    /* delete the implicit created TT */
    if valid-handle(iphTTBuffer) then
      delete object iphTTBuffer.
  end finally.

end procedure.

Oops, too fast... the error isn't raised so the catch block didn't go off.

I'm glad you all addressed the issue of how errors are handled when they are raised in a FINALLY block.  However, here we are not raising an error in the FINALLY block.  We are doing RETURN ERROR.  That is different.  That will always do the same thing, regardless of whether you have an ON ERROR, UNDO THROW directive.  It will return and then raise error in the caller.  

Besides all of that, it really has nothing to do with the original question which is why there seems to be a memory leak.  If you execute a DELETE statement on a handle, it will go away.  Then, even if you were to raise error in the same block, that does NOT undo the DELETE.  And in this case you are not even raising error in the block.  

I believe this has to do with the fact that the parameter is INPUT-OUTPUT.  Why is that?  If you are deleting the TT in the routine, it clearly is not meant to be returned to the caller.  If you change it to INPUT, the leak goes away.  

But still - this does seem like a bug to me.

But the help also mentions:

Note: RETURN ERROR error-object-expression immediately returns to the caller before throwing the error object. Unlike a direct THROW, it ignores any CATCH blocks or ON ERROR directives in effect at the time of the RETURN.

Although this doesn't mention FINALLY, if the CATCH is skipped you might as well assume that FINALLY is skipped as well.

My suggestion would be to move away from RETURN ERROR altogether and use structured error handling, ie. 

undo, throw new AppError('sorry', -1). 

This plays nicely with catch and finally.

Au contraire.  The FINALLY block is NOT skipped when you do RETURN ERROR.  The purpose of the FINALLY block is to always run regardless of the success or failure of the block.  And indeed it does run.  

So again, this issue has nothing to do with the error handling.

I stand corrected. I was jumping the conclusions.

Thanks Laura. Must I log this with Progress Support?

I've logged a case with Progress Support.

[quote user="jbijker"]

I've logged a case with Progress Support.

[/quote]

Do you have a defect number? I've just seen our code run into the same, now that throwing AppErrors around is becoming more popular. The input-output table-handle is not cleaned up when a Progress.Lang.AppError is thrown - when viewed in the debugger (dynamic object monitor) the leaked handle is shown as 'Deleted - pending'.

FYI - Using DynObjects logging to analyze this shows that this is clearly a bug.

This section depicts the scenario where no error is returned.  Note that, correctly, the table and buffer are Delete Pended within the FINALLY block and, when no error is returned, the line of code the actual deletion occurs on is line 12 (below):

RUN doTableLogic(INPUT-OUTPUT TABLE-HANDLE hTable, INPUT FALSE) NO-ERROR.

DYNOBJECTS     Created        TEMP-TABLE              Handle:1137 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 0)  Pool:<Session Pool>

DYNOBJECTS     Created        BUFFER                  Handle:1138 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 0) IMPLICIT Table:ttTest Pool:<Session Pool>

DYNOBJECTS     Delete Pending TEMP-TABLE              Handle:1137 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 31)

DYNOBJECTS     Delete Pending BUFFER                  Handle:1138 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 31) IMPLICIT

DYNOBJECTS     Deleted        TEMP-TABLE              Handle:1137 (D:\Work117\p05570_Untitled1.ped @ 12)

DYNOBJECTS     Deleted        BUFFER                  Handle:1138 (D:\Work117\p05570_Untitled1.ped @ 12) IMPLICIT

In this section you see that the table and buffer are not deleted until they're explicitly deleted(again) on line 46 in the scanLeaks procedure.

DYNOBJECTS     Created        TEMP-TABLE              Handle:1147 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 0)  Pool:<Session Pool>

DYNOBJECTS     Created        BUFFER                  Handle:1148 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 0) IMPLICIT Table:ttTest Pool:<Session Pool>

DYNOBJECTS     Created        Progress.Lang.Object    Handle:1149 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 26) Progress.Lang.AppError

DYNOBJECTS     Delete Pending TEMP-TABLE              Handle:1147 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 31)

DYNOBJECTS     Delete Pending BUFFER                  Handle:1148 (doTableLogic D:\Work117\p05570_Untitled1.ped @ 31) IMPLICIT

DYNOBJECTS     Deleted-by-GC  Progress.Lang.Object    Handle:1149 (D:\Work117\p05570_Untitled1.ped @ 12) Progress.Lang.AppError

DYNOBJECTS     Deleted        TEMP-TABLE              Handle:1147 (scanLeaks D:\Work117\p05570_Untitled1.ped @ 46)

DYNOBJECTS     Deleted        BUFFER                  Handle:1148 (scanLeaks D:\Work117\p05570_Untitled1.ped @ 46) IMPLICIT

This was logged as: OCTA-7459.

Note that the issue is specific to internal procedures. If you move the code to its own .p, the dynamic temp-table will get deleted when it returns, whether that was an error or not.

[quote user="Stefan Drissen"]

Do you have a defect number? I've just seen our code run into the same, now that throwing AppErrors around is becoming more popular. The input-output table-handle is not cleaned up when a Progress.Lang.AppError is thrown - when viewed in the debugger (dynamic object monitor) the leaked handle is shown as 'Deleted - pending'.

Thanks all - I've reached out to support - targeted to be fixed in 11.7.6 and 12.1 (both subject to change).