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

WELD-2747 Avoid calling SessionObjectReference.isRemoved() on every invocation. #2862

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

pferraro
Copy link
Contributor

@pferraro pferraro commented Jul 3, 2023

https://issues.redhat.com/browse/WELD-1234

SessionObjectReference.isRemoved() can be expensive. Let's avoid calling this per invocation, and instead return the fabricated toString() if the EJB was not found.
See wildfly/wildfly#16885 for context.

@manovotn
Copy link
Contributor

manovotn commented Jul 3, 2023

Hmm, this doesn't seem to cut it, the test is still failing @pferraro.
CI runs on latest WFLY patched to Weld in this PR, so the expectation is that is should succeed.

@pferraro
Copy link
Contributor Author

pferraro commented Jul 3, 2023

Ah - this is because the proxy invocation handler used by WildFly handles toString() via an earlier interceptor: https://github.com/wildfly/wildfly/blob/main/ejb3/src/main/java/org/jboss/as/ejb3/component/EJBComponentDescription.java#L972
which means that toString() will return successfully (i.e. it does not throw a NoSuchEJBException), even if a previous method threw an exception causing the bean instance to be discarded.

I'll fix the test assertion accordingly.

@manovotn manovotn changed the title Modify WELD-1234 fix to avoid calling SessionObjectReference.isRemoved() on every invocation. WELD-2747 Avoid calling SessionObjectReference.isRemoved() on every invocation. Jul 4, 2023
@manovotn
Copy link
Contributor

manovotn commented Jul 4, 2023

I've created a tracking issue for this - https://issues.redhat.com/browse/WELD-2747

Thanks for the PR @pferraro :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants