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

INT-3722: Fix Timing Issue TCP OG and Cached CF #1452

Merged
merged 1 commit into from May 27, 2015

Conversation

garyrussell
Copy link
Contributor

JIRA: https://jira.spring.io/browse/INT-3722

The TCP outbound gateway correlates replies based on the connection id.

When a CachingClientConnectionFactory is being used, the connection is
returned to the pool too early, and can be reused. It is possible that
the current thread then removes the "next" pending reply from the correlation map.

Add code to the CachingClientConnectionFactory so that the "self" close from the
connection (called after onMessage) is deferred and the actual close (return to cache)
is controlled by the gateway itself.

This mechanism will no longer be needed when INT-3654 is resolved (removal of the "self"
closing by connections). At that time, connection users (such as the gateway) will be
in complete control.

cherry-pick to 4.1.x

@garyrussell
Copy link
Contributor Author

@artembilan Since we're not building M1 until tomorrow now, we can change the JIRA to M1 if you are able to review this in time.

@garyrussell
Copy link
Contributor Author

fixed final modifier; squashed

JIRA: https://jira.spring.io/browse/INT-3722

The TCP outbound gateway correlates replies based on the connection id.

When a `CachingClientConnectionFactory` is being used, the connection is
returned to the pool too early, and can be reused. It is possible that
the current thread then removes the "next" pending reply from the correlation map.

Add code to the `CachingClientConnectionFactory` so that the "self" close from the
connection (called after `onMessage`) is deferred and the actual close (return to cache)
is controlled by the gateway itself.

This mechanism will no longer be needed when INT-3654 is resolved (removal of the "self"
closing by connections). At that time, connection users (such as the gateway) will be
in complete control.
@garyrussell
Copy link
Contributor Author

Added trap to avoid late add to deferredClosures; squashed.

@@ -155,6 +156,9 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
logger.debug("released semaphore");
}
}
if (this.connectionFactory instanceof CloseDeferrable) {
((CloseDeferrable) this.connectionFactory).closeDeferred(connectionId);
Copy link
Member

Choose a reason for hiding this comment

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

OK. I worried how we closed a connection before (or even right now artificially) and found that CachedConnection does that itself in the onMessage.

So, merging as is, but we reall should rethink how it works and who is responsible for what.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes; the responsibility is wrong; but fixing it would be too big a change for a point release (hence the reference to INT-3654).

Thanks

@artembilan artembilan merged commit 22fb524 into spring-projects:master May 27, 2015
@artembilan
Copy link
Member

Merged and cherry-picked

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.

None yet

2 participants