FWD » Core Development » Base Language

In #8912-61, (and others), it was found that at least in this test FWD does not process the FINALLY blocks transaction properly:

do transaction:
      for each book: delete book. end.
      create book.
      book.book-id = 1.
      book.book-title = "abc".
      book.isbn = "1".
      create book.
      book.book-id = 2.
      book.book-title = "abc".
      book.isbn = "2".
      release book.
end.
def buffer bbook for book.
for each bbook exclusive-lock:
   message bbook.book-id bbook.book-title.
   finally:
      delete bbook.
   end finally.
end.

The #8593-113 may be related to how we managed buffers/transaction in FINALLY blocks.

• what happens if the block to which the FINALLY (or CATCH) is attached is not a FULL tx, but a sub-tx?
• what happens if the block is exiting via a undo operation?
• what happens if there is an error in the FINALLY block - can its tx be undone?
• anything else we can think on the way.

These are the current classes that implement Finalizable and have an implementation for the finished method, classified by package:

persist

BufferManager------------------------no support
CompoundQuery------------------------no support
DynamicTablesHelper.Context----------no support
OutputDataSetCopier------------------no support
OutputDataSetHandleCopier------------no support
OutputTableCopier--------------------no support
OutputTableHandleCopier--------------no support
Persistence.Context------------------no support
PreselectQuery-----------------------no support
RecordBuffer-------------------------no support
TemporaryBuffer.Multiplexer----------no support
TemporaryBuffer.NoUndoValidator------no support
TxWrapper----------------------------specific to finally
TxWrapper.WorkArea-------------------no support

SavepointManager---------------------no support
UniqueTracker.Context----------------no support

GenericFrame-------------------------no support
GenericFrame.AggregateHandler--------no support
GenericFrame.EditingCleaner----------no support
LogicalTerminal----------------------no support
LogicalTerminal.ScopeProcessor-------no support

Accumulator--------------------------no support
decimal.PrecisionResetter------------no support
DeferredDeletablesManager------------no support
StopAfterTimer.Task------------------no support
Stream-------------------------------no support
Text.SesitivityResetter--------------no support
UnnamedStreams.InputCloser-----------no support
UnnamedStreams.OutputCloser----------no support

I've found something that is not related to finally blocks, but should be addressed:
for each from #8944-1

for each bbook exclusive-lock:
   message bbook.book-id bbook.book-title.
   delete bbook.
end.

Can you post the full example for #8944-5?

This is the test case that replicates the issue:

do transaction:
      for each p_table_1: delete p_table_1. end.
      create p_table_1.
      p_table_1.number = 1.
      create p_table_1.
      p_table_1.number = 2.
      release p_table_1.
end.
def buffer pbuf for p_table_1.
for each pbuf exclusive-lock:
   message pbuf.number.
   delete pbuf.
end.

I have been able to replicate this issues only with the tables that have indices.

I think this is related to intra-session dirty manager (?) Maybe you should embed the for each in a separate do transaction to isolate the two transaction (one that creates the data and the second that deletes). Anyway, this is out-of-scope, so don't lose time on this now.

Alexandru, I've been analyzing #8593-113 and #8593-111, I am failing to see the issue there, for me both test cases seem to behave as expected.

In #8593-111, if you comment the find first that has a comment, in FWD it will work while in 4GL it won't.

define temp-table tt field f1 as int.  
do transaction:
    create tt.
    tt.f1 = 1.
end.
for each tt:
    define buffer btt2 for tt.
    find first btt2 no-error.
    btt2.f1 = 10.
    finally:
        btt2.f1 = 20.
        message "abc".
    end.
end.

Tested with trunk/15463.

Hmm I think the error is more subtle. IIRC, the tests were trying to show a conceptual problem in regard to buffer scoping. The scope of the buffer was closing if defined in the for each, but it could be usable in the finally. So you may end up using a buffer with its scope already closed. I don't know right know how to make a complete test-case.

define temp-table tt field f1 as int.  
do transaction:
    create tt.
    tt.f1 = 1.
end.
for each tt:
    define buffer btt2 for tt.
    find first btt2 no-error.
    btt2.f1 = 10.
    finally:
        btt2.f1 = 20.
    end.
end.
message btt2.f1.// without this line, the error is thrown

Artur Școlnic wrote:

[...]
I looks like the scope of the btt2 is limited to the for each only if it is not referenced outside the for each.
In both cases (referencing the buffer outside the for each and not) the behavior in FWD and 4GL is the same.

This is normal. But my point is how FWD is handling the scope of btt2 inside finally. You can see that RecordBuffer has an openScopeCount. Before reaching finally, the scope count reaches 0. But if you do a find first in the finally, you end up using a buffer without its scope being opened. So, I think the changes done in finally won't be committed (?) or something like that. You will need to do some FWD debugging for this.

Artur, please look into #6667-1167. That is a fix that was dedicated to fixing buffer problems with FINALLY. But as Tomasz noted, it may be flawed and removed. The right way to fix that is #8944.
My point: attempt to revert the change in #6667-1167 and retest #8593-113 and #8593-111. That patch will be reverted anyway, so we need to be reactive with that and fix it properly in 8944.

Eduard [es], you have seen issues in a unit test suite if you apply:

=== modified file 'src/com/goldencode/p2j/persist/orm/ChangeSet.java'
--- old/src/com/goldencode/p2j/persist/orm/ChangeSet.java    2024-03-11 10:04:44 +0000
+++ new/src/com/goldencode/p2j/persist/orm/ChangeSet.java    2024-09-26 15:41:06 +0000
@@ -17,6 +17,7 @@
 ** 005 TJD 20230303 Allow resetting changeset state and changeset changes roll-up 
 **     TJD 20230705 Make sure resetted changesets fit into structures  
 **     TJD 20231019 fix resetChangeset function name, remove unused variable
+**     TJD 20240926 Revert AL2 20240517 change, as its causing regression 
 */

 /*
@@ -338,16 +339,8 @@

          if ((baselineState & CACHED) != CACHED && (startingState & CACHED) == CACHED)
          {
-            if (dmo.isInUse()) 
-            {
-               // the DMO is in use so we can't evict it; make sure it is still marked as cached
-               liveState.state |= CACHED;
-            }
-            else
-            {
-               // rolled back DMO must be removed from session cache
-               session.evict(dmo);
-            }
+            // rolled back DMO must be removed from session cache
+            session.evict(dmo);
          }
          else if ((baselineState & CACHED) == CACHED && (startingState & CACHED) != CACHED)
          {

This is basically the fix that worked-around actually fixing FINALLY.

Artur/Eduard: please work together to have this fixed. Eduard, please check if you can make a better recreate than #8593-113 and #8593-111 and help Artur have a clear material to work on. In the mean-time, lets check what is left to be cleared from #8944-4.

Please see #9139-63 and #9139-64.
The root cause is that after parameter in saveRowChangesImpl2 is changed in do block and it is not staged for undo because after is no-undo temp-table.

When a exception inside do block, the Session for _temp database is rollbacked, so dmo for after is rollbacked, but after buffer itself is not undone.

In 9139a, rev. 15593 - 15594. I attempt to do force undo of the after buffer in case a exception is thrown. Since the change to after was made artificially in fwd code.

Now once we are in doBlock2 from #8944-21 example, an error is thrown and the primary database is being rollbacked. And also stat2 record is removed from Session cache with #8944-18 in place.

After exiting of doBlock2 block, second attempt to find dirty stat2 record will fail in fwd, but not in 4gl.

stat2 record created previously still exists in Dirty database, and will be retrived in RAQ.executeImple with dirtyContext.getDirtyInfo. But processDirtyResults it will still try to load the record from primary database with persistence.load(buffer.getDMOImplementationClass(), id, lockType, 0L, updateLock);.

Obviously it will not find it neither in Session cache nor in primary database, because of the whole story from #8944-21.

Coming back to the issue this task was created for, I analyzed the test case from #8944-1 and came to a conclusion.
For the finally block, new instances of TxWrapper objects are created for each active database, but when it comes to transaction management, the session is checked, which is common for both enclosing blocks transactions and finally block transactions.
I think we need to further differentiate between a finally transaction and an enclosing block transaction, a different TxWrapper is not enough. Maybe we could keep track of different 'types' of transactions in Session. Another possible solution is to have separate sessions, but that may be too radical of an approach.

do transaction:
      for each book: delete book. end.
      create book.
      book.book-id = 1.
      book.book-title = "abc".
      book.isbn = "1".
      create book.
      book.book-id = 2.
      book.book-title = "abc".
      book.isbn = "2".
      release book.
end.
define temp-table tt field f1 as int.
def buffer bbook for book.
for each bbook exclusive-lock:
   message bbook.book-id bbook.book-title.
   finally:
       create tt.
       tt.f1 = 1.
       release tt.
   end finally.
end.

** Requested to persist DMO Tt... A different DMO of type ... is already bound to this session with the same recid

As far as I know, a full transaction is started by a block only if a transaction is not already active, and if it is, a subtransaction is started, following this logic, the finally block can't ever be a full transaction inside a block that is a full transaction or even a sub transaction. I am bringing this up because in the test cases I am working with, finally attempts to start a full transaction.

I was trying to analyze transactions for the examples above using

FIND FIRST _Trans WHERE _Trans-State <> ? NO-LOCK NO-ERROR.
message _Trans._Trans-Num.

_TRANS virtual system table is not supported by fwd

Correct

• exclusive-lock forces a transaction to be opened, regardless of the code being executed in te associated block.
• finally opens a new transaction regardless of whether the associated block already opened one or not.
• One transaction is opened for the finally for each iteration of the block.

• finally uses the transaction opened by the outer(full) transaction.
• The same transaction is being used for the whole duration of the outer(full) transaction.

For this investigation, I considered that a different _TRANS._Trans-Num, represents a different transaction being opened.

Further analysis into finally blocks and transactions based on the example in #8944-1.

A scenario without a finally block:
1. For each begins a transaction, scope start.
2. iterateWorker processed the commit.
3. processFinalizables begins a transaction to iterate all the finalizables in the block.
4. On the second iteration the iterateWorker will process the commit, same as step 2.
5. processFinalizables begins a transaction to iterate all the finalizables in the block.
6. For each closes the transaction with a commit, pop scope.

A scenario with a finally block that opens a transaction:
1. For each begins a transaction, scope start.
2. iterateWorker processed the commit.
3. processFinalizables begins a transaction to iterate all the finalizables in the block.
4. Finally block attempts to begin it's transaction, but fails because one is already active.

A scenario with a finally that does not open a transaction:
1. For each begins a transaction, scope start.
2. iterateWorker processed the commit.
3. processFinalizables begins a transaction to iterate all the finalizables in the block.
4. Finally block rollbacks, so the transaction is closed.
5. iterateWorker attempts to process the commit, but there is no current transaction opened.

It is worth mentioning that in 4GL not all finally blocks start transactions, but only those that actually need one, for example the ones that delete/create records.
In FWD, any non empty finally block will either open a transaction if needed, or commit/rollback because of the finalizables processing inside the finally block.
Another big difference is that in 4GL, at the beggining of the finally execution, the transaction opened by the iterating block is closed, in FWD, the transaction opened by the processFinalizables is still open, that actully is the root cause of the problem in #8944-1.

Considering that in 4GL, finally is executed at the end of each iteration of the enclosing block, extracting the finally execution in the while (true) loop of the forEachWorker seems to be a closer implementation.
Constantin, do you have an opinion on this?

deleted

• iterateWorker when the block is iterating
• processRetry in case of retry
• popScope for the 'last' iteration (when the block exits)

So, are all cases being 'faulty' or just the popScope one?

Constantin Asofiei wrote:

So, are all cases being 'faulty' or just the popScope one?

I have seen issues only with iterateWorker, the execution does not reach popScope.

There is another potential issue that maybe should be discussed.
In notifyMasterFinish and processFinalizables, in the finalizables collections, there are 2 TxWrapper objects for the same database with the same persistence context for each active db, one added for the enclosing block and one for the finally.
This is the precursor to one of the transaction issues. 8944a works around it, but I am curious if that was intended.

Artur, I need to review this together with #9004. I'll put this in the queue, but is not just review, I need to recall this behavior and re-test it.

I will take a look at this one, since there CATCH blocks also need transaction support. And there is a good chances that this support for this could be common.

Artur Școlnic wrote:

As far as I know, a full transaction is started by a block only if a transaction is not already active, and if it is, a subtransaction is started, following this logic, the finally block can't ever be a full transaction inside a block that is a full transaction or even a sub transaction. I am bringing this up because in the test cases I am working with, finally attempts to start a full transaction.

I have the following stack trace when we execute the finally block of the forEach in #8944-1 from the stack trace.

BlockManager.doTerminationBlockWorker(BlockManager$WorkArea, BlockManager$TransactionType, String, String, boolean, Block) line: 11019    
BlockManager.lambda$processForBody$1(BlockManager$WorkArea, BlockManager$TransactionType, BlockDefinition, Block) line: 9709    
0x00007d2d58965b30.run() line: not available [local variables unavailable]    
TransactionManager.processFinallyBlock(TransactionManager$WorkArea, BlockDefinition) line: 5117    
TransactionManager.lambda$iterateWorker$2(TransactionManager$WorkArea, BlockDefinition) line: 6799    
0x00007d2d589663c0.run() line: not available [local variables unavailable]    
TransactionManager.executeBlockTermination(TransactionManager$WorkArea, BlockDefinition, Runnable, boolean) line: 4994    
TransactionManager.iterateWorker(TransactionManager$WorkArea) line: 6799    
TransactionManager.blockSetup(TransactionManager$WorkArea) line: 5229    
TransactionManager$TransactionHelper.blockSetup() line: 9247    
BlockManager.forEachWorker(BlockManager$TransactionType, P2JQuery, String[], String, ToClause, LogicalOp, OnPhrase[], Block) line: 11983    

ForEach block from #8944-1, opens it's own transaction and sets the TM.WorkArea.transLevel = 2. On the call stack there is a call to TM.processFinallyBlock method, where the value of TM.WorkArea.transLevel is resetted to -1. I think because of this in fwd it will try to execute a new transaction.

Isn't a new transaction for the finally block expected?

Artur Școlnic wrote:

Isn't a new transaction for the finally block expected?

I created the a test cases to look at the transaction that are being created in CATCH and FINALLY blocks.

do transaction:
      for each book: 
         delete book. 
      end.

      for each test1: 
         delete test1. 
      end.

      create book.
      book.book-id = 1.
      book.book-title = "abc".

      create book.
      book.book-id = 2.
      book.book-title = "abc".
      release book.

      create test1.
      test1.f1 = "abc".
      release test1.
end.

def var h as handle.

FIND FIRST _Trans WHERE _Trans-State <> ? NO-LOCK NO-ERROR.

if available(_Trans) then do:
   message "before forEach " _Trans._Trans-Num.
end.

def buffer bbook for book.
for each bbook exclusive-lock:   
   for each _Trans WHERE _Trans-State <> ? NO-LOCK:
      message "book forEach " _Trans._Trans-Num.
   end.

   for each test1 transaction:
      delete test1.

      for each _Trans WHERE _Trans-State <> ? NO-LOCK:
         message "test1 forEach " _Trans._Trans-Num.
      end.
   end.
   h = h:next-sibling.

   CATCH ex1 AS Progress.Lang.Error :
         for each _Trans WHERE _Trans-State <> ? NO-LOCK:
        message "catch " _Trans._Trans-Num.
      end.

      create test2.
      test2.f2 = "asd".
      release test2.
   END CATCH.

   finally:
      for each _Trans WHERE _Trans-State <> ? NO-LOCK:
        message "finally " _Trans._Trans-Num.
      end.

      delete bbook.
   end finally.
end.

With the following results after running, it represents the the current block from which the message is outputted and transaction number:

book forEach  112
test1 forEach  112
catch  113
finally  114
book forEach  115
test1 forEach  115
catch  116
finally  117

It seems that a new transaction is created at the beginning of each iteration of for each bbook block, the same transaction is used inside the for each test1 block.
And it is closed at the the end of the iteration.
CATCH block and FINALLY block each creates it's own transaction, which is closed after execution of the respective block.

If the FINALLY is more deeply nested (the full transaction is in a more enclosing block, not in the immediately containing block) then I would expect that there is NOT a separate transaction at the FINALLY (though perhaps it is a separate sub-transaction). This should be tested.

Greg Shah wrote:

If the FINALLY is more deeply nested (the full transaction is in a more enclosing block, not in the immediately containing block) then I would expect that there is NOT a separate transaction at the FINALLY (though perhaps it is a separate sub-transaction). This should be tested.

That's right. If I have a nested repeat block with a finally, and in this case there not separate transaction open at FINALLY, or CATCH.

for each bbook exclusive-lock:   
   for each _Trans WHERE _Trans-State <> ? NO-LOCK:
      message "book forEach " _Trans._Trans-Num.
   end.

   repeat i = 1 to 5 transaction:
      message i.
      for each _Trans WHERE _Trans-State <> ? NO-LOCK:
         message "repeat " _Trans._Trans-Num.
      end.

      finally:
        for each _Trans WHERE _Trans-State <> ? NO-LOCK:
          message "finally " _Trans._Trans-Num.
        end.

        create test1.
        test1.f1 = "asf".
        release test1.
      end finally.
   end.
end.

Eduard, in 4GL there is an implicit FULL transaction created when EXLCUSIVE-LOCK is used or a CREATE <buffeR> is used. So is not mandatory to have TRANSACTION for a FULL transaction to exist.

Constantin Asofiei wrote:

Eduard, in 4GL there is an implicit FULL transaction created when EXLCUSIVE-LOCK is used or a CREATE <buffeR> is used. So is not mandatory to have TRANSACTION for a FULL transaction to exist.

I understand. I was looking if a new transaction will be opened in the nested repeat block, if a transaction is already opened by the for each bbook EXLCUSIVE-LOCK block. But it seems that it is not the case.

According to note #8944-26 and my observation in FWD.
Transaction is opened in TM.PushScope, through TxWrapper.scopeStart.
For iterative blocks, in iterateWorker the transaction is committed if there isn't any rollback and notifyMasterFinish method is called.

In notifyMasterFinish, the iterate method of each TxWrapper is called. In iterate method transaction is close, and right after that is opened again.

My idea is to just close the transaction in iterate method, and open transaction again for the next operation only after execution of CATCH and FINALLY.

I have added a fix for transaction management in 4602a, rev. 15880.

The patch from #9803-46 solved the initial problem this task was opened for, considering that changes from 4602a will reach trunk eventually and will reach 7156e shortly, I think #8944 is redundant and 8944a can be killed.

Artur Școlnic wrote:

The patch from #9803-46 solved the initial problem this task was opened for, considering that changes from 4602a will reach trunk eventually and will reach 7156e shortly, I think #8944 is redundant and 8944a can be killed.

Does it solve the #8944-24 test?

No, it does not solve the test cases from this task, the problem is that there are multiple ways to get to end up in a broken transaction state, the particular problem this task was created for, some failing customer unit tests, was solved, this just means that the test case was not the exact replicate of the original problem.

Artur Școlnic wrote:

No, it does not solve the test cases from this task, the problem is that there are multiple ways to get to end up in a broken transaction state, the particular problem this task was created for, some failing customer unit tests, was solved, this just means that the test case was not the exact replicate of the original problem.

But is still a problem :). Ok, I will move the discussion about this to #4602.