Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Report: DataSource.getConnection() blows up on transaction with wrong status/FISH-7451 #6274

Closed
thehpi opened this issue May 1, 2023 · 4 comments
Assignees
Labels
Status: Accepted Confirmed defect or accepted improvement to implement, issue has been escalated to Platform Dev Type: Bug Label issue as a bug defect

Comments

@thehpi
Copy link

thehpi commented May 1, 2023

Brief Summary

When using two entity listeners where one is using @transactional(Transactional.TxType.REQUIRES_NEW) this causes problems when doing DataSource.getConnection() after observing an event using @observes(during = TransactionPhase.AFTER_SUCCESS).

See the reproducer: https://github.com/thehpi/payara-after-success

Expected Outcome

I expect to always be able to get a connection from a datasource, regardless of what other processing has been done before using entity listeners or observers.

Current Outcome

An exception is thrown:

RAR5029:Unexpected exception while registering component
java.lang.IllegalStateException: Transaction is not active in the current thread.
at com.sun.enterprise.transaction.JavaEETransactionImpl.checkTransationActive(JavaEETransactionImpl.java:756)
at com.sun.enterprise.transaction.JavaEETransactionImpl.enlistResource(JavaEETransactionImpl.java:692)
at com.sun.enterprise.transaction.jts.JavaEETransactionManagerJTSDelegate.enlistDistributedNonXAResource(JavaEETransactionManagerJTSDelegate.java:298)
at com.sun.enterprise.transaction.JavaEETransactionManagerSimplified.enlistResource(JavaEETransactionManagerSimplified.java:469)
at com.sun.enterprise.resource.rm.ResourceManagerImpl.registerResource(ResourceManagerImpl.java:152)
at com.sun.enterprise.resource.rm.ResourceManagerImpl.enlistResource(ResourceManagerImpl.java:112)
at com.sun.enterprise.resource.pool.PoolManagerImpl.getResource(PoolManagerImpl.java:211)
at com.sun.enterprise.connectors.ConnectionManagerImpl.getResource(ConnectionManagerImpl.java:360)
at com.sun.enterprise.connectors.ConnectionManagerImpl.internalGetConnection(ConnectionManagerImpl.java:307)
at com.sun.enterprise.connectors.ConnectionManagerImpl.allocateConnection(ConnectionManagerImpl.java:196)
at com.sun.enterprise.connectors.ConnectionManagerImpl.allocateConnection(ConnectionManagerImpl.java:171)
at com.sun.enterprise.connectors.ConnectionManagerImpl.allocateConnection(ConnectionManagerImpl.java:166)
at com.sun.gjc.spi.base.AbstractDataSource.getConnection(AbstractDataSource.java:113)

Reproducer

https://github.com/thehpi/payara-after-success

Operating System

MacOSX Ventura 13.3.1

JDK Version

openjdk 11.0.17 2022-10-18

Payara Distribution

Payara Server Full Profile: 6.2023.2

@thehpi thehpi added Status: Open Issue has been triaged by the front-line engineers and is being worked on verification Type: Bug Label issue as a bug defect labels May 1, 2023
@Elifzeynepedman Elifzeynepedman self-assigned this May 2, 2023
@thehpi
Copy link
Author

thehpi commented May 2, 2023

After debugging I found the following. When getting the connection from the datasource in the ResourceManagerImpl at line 150 the current transaction is checked. This transaction is retrieved using ComponentInvocation.getTransaction().

            if (tran != null) {
 	        tm.enlistResource(tran, handle);

If there is a transaction it will call enlistResource which at some point checks the transaction status and a COMMITTED tx throws an exception.

I noticed that when the EntityListener2 is not there then the 'tran' variable is null and the transaction status is not checked and no exception is thrown. So why is the it null?

I found that when using

    @Transactional(Transactional.TxType.REQUIRED)

then this interceptor class is used

    org.glassfish.cdi.transaction.TransactionalInterceptorRequired

when using

    @Transactional(Transactional.TxType.REQUIRES_NEW)

then this interceptor class is used

    org.glassfish.cdi.transaction.TransactionalInterceptorRequiresNew

So I investigated those.

This is what is happening: the TransactionalInterceptorRequiresNew will suspend the current transaction (if its there) and create a new one in which the code is executed. After executing the code the current transaction is suspended.

But the thing is, this interceptor also registers the transaction on the ComponentInvocation. And when it suspends the transaction after running the actual code it not only suspends the transaction but it also registeres this transaction on the ComponentInvocation, even if there was none before the interceptor started.

And this seems to be the problem. Before running EntityListener2 there is no tx on ComponentInvocation, but when its done there is. Ultimately this is then causing the exception described above.

So it looks like a bug but I'm not sure. The TransactionManager interface has multiple implementations and I don't know if payara always uses the same implementation. Currently it is using the

    com.sun.enterprise.transaction.TransactionManagerHelper

and the resume method also calls preInvokeTx() which is setting the tx on ComponentInvocation.

public void resume(Transaction tobj)
        throws InvalidTransactionException, IllegalStateException,
        SystemException {
    transactionManager.resume(tobj);
    preInvokeTx(false);
}

A fix could be to 'remember' if there was a tx on the ComponentInvocation and if not set it to null after suspending the tx.

@Elifzeynepedman Elifzeynepedman changed the title Bug Report: DataSource.getConnection() blows up on transaction with wrong status Bug Report: DataSource.getConnection() blows up on transaction with wrong status/FISH-7451 Jun 21, 2023
@Elifzeynepedman
Copy link

Hi @thehpi ,

This has been escalated this to the platform development team as FISH-7451.

Best Regards,
Elif

@fturizo fturizo added Status: Accepted Confirmed defect or accepted improvement to implement, issue has been escalated to Platform Dev and removed Status: Open Issue has been triaged by the front-line engineers and is being worked on verification labels Jul 17, 2023
@aubi
Copy link
Contributor

aubi commented Jul 19, 2023

The PR with fix was merged, thank you!

@Elifzeynepedman
Copy link

Hi @thehpi,

This issue has been fixed in Payara 6.2023.8 which will be available in the next release.

Thank you,
Elif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Confirmed defect or accepted improvement to implement, issue has been escalated to Platform Dev Type: Bug Label issue as a bug defect
Projects
None yet
Development

No branches or pull requests

4 participants