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.

Sign up for GitHub

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

Jump to bottom

[WFLY-15047] EJB (Un)marshaller resource leak #14498

Merged
merged 1 commit into from Jul 23, 2021
Merged

[WFLY-15047] EJB (Un)marshaller resource leak #14498

merged 1 commit into from Jul 23, 2021

Conversation

boris-unckel
Copy link
Contributor

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Jul 21, 2021
@chengfang
Copy link
Contributor

marshaller and unmarshaller have a finish() method that does the cleanup. In fact, its close() method just calls finish(). So adding a call to close() will duplicate what's already done by finish() method for places where finish() is already called. So I think it's better to add a finally clause to make sure finish() is always called for cleanup.

@boris-unckel
Copy link
Contributor Author

@chengfang I should have checked that :-( I suggest to remove the finish() to avoid the duplication. try-with-resource is caring for the correct Exception/Suppressed Ex handling including to avoid NullPointer.

@boris-unckel boris-unckel force-pushed the WFLY-15047_ejb_close_resource branch from e02031d to a4709d0 Compare July 22, 2021 07:13
@boris-unckel
Copy link
Contributor Author

I have found a second issue here: finish() and flush() had the wrong order and flush() was sometimes in use, sometimes not. It's fixed now.

@boris-unckel
Copy link
Contributor Author

Hello @bstansberry Tests passed for this PR and it's approved. Could you check and merge, please?

@bstansberry bstansberry merged commit 6b9d980 into wildfly:master Jul 23, 2021
@boris-unckel boris-unckel deleted the WFLY-15047_ejb_close_resource branch July 24, 2021 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chengfang chengfang chengfang approved these changes

Assignees
No one assigned
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
Milestone
No milestone
3 participants
@boris-unckel @chengfang @bstansberry