Message Duplication in AbstractCorrelatingMessageHandler [INT-2937]

[spring-operator]

Stefan Ferstl opened INT-2937 and commented

#6726 tried to fix an issue where a reaper could reap an already released message group. The solution was to compare the lastModified time stamps of the message group in AbstractCorrelatingMessageHandler and the reaper's copy of the same message group. Since the lastModified timestamp's resolution is only milliseconds, the issue can still occur when there is some load on the application. I attached a unit test that reveals the problem quite quickly. The test does almost never succeed (at least on my Intel Core i7-2600K PC).

To solve this problem I suggest to introduce a version number (and maybe remove the lastModified timestamp) on the message group which increases each time the group changes. AbstractCorrelatingMessageHandler#forceComplete() can use this version number for comparison instead of the lastModified timestamp.

---

Affects: 2.2.1

Attachments:

• spring-integration-core.zip (3.67 kB)

2 votes, 5 watchers

[spring-operator]

Gary Russell commented

(and maybe remove the lastModified timestamp)

We can't do that because group expiration can be based on that (if timeoutOnIdle is true).

introduce a version number

Schema changes (such as for the JDBC store) are difficult on point releases (but it could be considered for 3.0).

I have to think about it some more, but perhaps something like this would work...

long nowMinusOneMillis = System.currentTimeMillis();
Thread.sleep(1);
boolean removeGroup = true;
try {
	lock.lockInterruptibly();
	try {
		/*
		 * Need to verify the group hasn't changed while we were waiting on
		 * its lock. We have to re-fetch the group for this. A possible
		 * future improvement would be to add MessageGroupStore.getLastModified(groupId).
		 */
		MessageGroup messageGroupNow = this.messageStore.getMessageGroup(
				group.getGroupId());
		long lastModifiedNow = messageGroupNow.getLastModified();
		if (group.getLastModified() == lastModifiedNow && lastModifiedNow <= nowMinusOneMillis {

We could avoid the millisecond sleep on every group by doing it once in the Callback and pass nowMinusOneMillis as a parameter into forceComplete.

[spring-operator]

Gary Russell commented

Nope; that won't work - while it might make your test pass, it doesn't detect if the group changed in the same millisecond, but that millisecond was more than 1 ago :-(

[spring-operator]

Gary Russell commented

Actually, the more I think about it - given that we re-fetch the group to get it's lastUpdateTime within the scope of the lock - can't we simply expire the newly fetched version of the group...

garyrussell@fd433bc

??

[spring-operator]

Stefan Ferstl commented

Yes, I think this will work.

[spring-operator]

Gary Russell commented

PR: #751

[spring-operator]

Gary Russell commented

This is now resolved in the recent 2.2.2.RELEASE.