Skip to content

Protect JMS connection creation against prepareConnection errors#29116

Merged
simonbasle merged 2 commits intospring-projects:mainfrom
lenoch7:SingleConnectionFactory-Reconnection
Feb 28, 2023
Merged

Protect JMS connection creation against prepareConnection errors#29116
simonbasle merged 2 commits intospring-projects:mainfrom
lenoch7:SingleConnectionFactory-Reconnection

Conversation

@lenoch7
Copy link
Contributor

@lenoch7 lenoch7 commented Sep 9, 2022

The creation of new JMS connection (after reset) is done in local method field (instead of instance field directly) to prevent situation, when connection is not correctly configured/prepared in case when JMSException from prepare() method is thrown.

See gh-29115

@pivotal-cla
Copy link

@lenoch7 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@lenoch7 Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 9, 2022
@lenoch7
Copy link
Contributor Author

lenoch7 commented Sep 9, 2022

I made next commit (I am very sorry, squash can be done later of course). This change should prevent to leave reference to SingleConnectionFactory (via exception listener) in invalid connection in case when connection.start() method throws JMSException.

Copy link

@valituguran valituguran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

The creation of new JMS connection (after reset) is done in local
method field (instead of instance field directly) to prevent
situation, when connection is not correctly configured/prepared in
case when JMSException from prepare() method is thrown.

See gh-29115
@lenoch7
Copy link
Contributor Author

lenoch7 commented Dec 16, 2022

I squash my two commits and rebase it on current HEAD. Is something still missing in this PR? What I can do?

@poutsma poutsma added the in: messaging Issues in messaging modules (jms, messaging) label Jan 30, 2023
Copy link
Contributor

@simonbasle simonbasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @jhoeller what do you think?

@simonbasle simonbasle linked an issue Feb 6, 2023 that may be closed by this pull request
@simonbasle simonbasle changed the title Fix creation of new JMS connection Protect JMS connection creation against prepareConnection errors Feb 6, 2023
@simonbasle simonbasle self-assigned this Feb 6, 2023
@simonbasle simonbasle added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 6, 2023
@simonbasle simonbasle merged commit 4cc02fe into spring-projects:main Feb 28, 2023
@simonbasle simonbasle added this to the 6.0.6 milestone Feb 28, 2023
simonbasle pushed a commit that referenced this pull request Feb 28, 2023
This commit uses a local variable for the creation of a new JMS
Connection so that a rare failure in prepareConnection(...) does not
leave the connection field in a partially initialized state.

If such a JMSException occurs, the intermediary connection is closed.
This commit further defends against close() failures at that point,
by logging the close exception at DEBUG level. As a result, the original
JMSException is always re-thrown.

See gh-29116
Closes gh-30051
@lenoch7 lenoch7 deleted the SingleConnectionFactory-Reconnection branch March 8, 2023 13:28
@lenoch7
Copy link
Contributor Author

lenoch7 commented Mar 8, 2023

Thank you for merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: messaging Issues in messaging modules (jms, messaging) type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SingleConnectionFactory - reconnection problem (AMQ Broker)

6 participants