Implicitly created TTs not deleted when returning with an error - Forum - OpenEdge General - Progress Community

Implicitly created TTs not deleted when returning with an error

 Forum

Implicitly created TTs not deleted when returning with an error

This question is not answered

Whenever you pass table handles to a procedure I'm aware that a new implicit TT is created when the procedure is called, and you need to do handle the cleanup of this implicit created TT. This cleanup works well if no error is returned.

However I've noticed that if you return with an error from the procedure, with the cleanup code in a finally block, for some reason this implicit created TT is NOT deleted, giving memory leaks.

See below code to reproduce. If you want to run without raising an error just change the second parameter on line 9 from INPUT TRUE to INPUT FALSE.

In short we have a temp-table, and we're passing the temp-table handle over to procedure doTableLogic. Inside this procedure we have code that handles the clean-up of the implicit created temp-table in the FINALLY block. After this we scan all buffer handles in the session to see if we have any buffer handles remaining (procedure scanLeaks), and if we find one we report on it & delete it. If we run the procedure without raising an error no memory leaks are reported, but whenever you run it and it returns with an error we get a memory leak.

Can anyone explain this, or ways to handle the cleanup better, or is this a Progress bug?

Tested on OE 11.3, 11.6 & 11.7.


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.

  DEFINE VARIABLE cError AS CHARACTER   NO-UNDO.

  MESSAGE "iphTTBuffer:" iphTTBuffer.

  IF iplReturnError THEN
    ASSIGN cError = "sorry".

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

    IF cError > "" THEN
      RETURN ERROR cError.

  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.


All Replies
  • 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.