Re: [cu-dev] Concurrency DeploymentDescriptorTests failing

[arjan tijms <arjan.tijms@xxxxxxxxx>]

Hi Petr,

Thanks! I see now that the code used to be "extra", but that in Concurrency 3 that same code still applies, but only when explicitly set to do that now. Otherwise it should be skipped,  E.g. in XML like this:

  <context-service>
    <name>java:global/concurrent/ContextD</name>
    <cleared>IntContext</cleared>
    <propagated>Application</propagated>
    <propagated>StringContext</propagated>
    <unchanged>Transaction</unchanged>
  </context-service>

Kind regards,

Arjan Tijms

On Mon, Jun 13, 2022 at 10:08 PM Petr Aubrecht <aubrecht@xxxxxxxxxxxx> wrote:

Hi Arjan,

the new feature in Concurrency 3 is requirement is to keep transaction unchanged (tested by ContextD), so it is necessary to change the condition:

https://github.com/payara/Payara/blob/Payara6/appserver/concurrent/concurrent-impl/src/main/java/org/glassfish/concurrent/runtime/ContextSetupProviderImpl.java#L394

if (contextClear.contains(CONTEXT_TYPE_WORKAREA) && transactionManager != null) {

     // clean up after user if a transaction is still active

...

E.g. the transaction is finished only if required.

Petr

On 6/13/22 17:40, arjan tijms wrote:

Hi,

The concurrency TCK currently has test code for this in the "DeploymentDescriptorTests" (simplified):

@Resource
 UserTransaction tx; 

        ContextService contextSvc = InitialContext.doLookup("java:global/concurrent/ContextD");

        tx.begin();
        try {
            checkContextAndGetTransactionStatus = contextSvc.contextualCallable(() -> {
                return tx.getStatus(); // unchanged
            });

            int status = checkContextAndGetTransactionStatus.call();
            assertEquals(status, Status.STATUS_ACTIVE);
        } finally {
            tx.rollback();
        }

In GlassFish, this fails as the TX is committed after the call to the ContextService.

 What happens is that after the callable is invoked, there is a reset; contextSetupProvider.reset(contextHandleForReset);

Which eventually does this:

if (transactionManager != null) {
            // clean up after user if a transaction is still active
            // This is not required by the Concurrency spec
            Transaction transaction = transactionManager.getCurrentTransaction();
            if (transaction != null) {
                try {
                    int status = transaction.getStatus();
                    if (status == Status.STATUS_ACTIVE) {
                        transactionManager.commit();
                    } else if (status == Status.STATUS_MARKED_ROLLBACK) {
                        transactionManager.rollback();
                    }
                } catch (Exception ex) {
                    Logger.getLogger(this.getClass().getName()).log(Level.SEVERE, ex.toString());
                }
            }
          transactionManager.clearThreadTx();
        }

As the comment already says, it's not required to commit / cleanup, so to pass this test which assumes the TX stays open, I could simply remove the cleanup, or maybe not call reset in the first place, or perhaps add a new alternative reset method.

Do any of the GlassFish or Concurrency RI committers have any thoughts on this? The behaviour could be changed at both places, and may have some consequences elsewhere. 

Kind regards,

Arjan Tijms

_______________________________________________
cu-dev mailing list
cu-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cu-dev