Skip to content

GH-3716: Fix wait for init redisMessageListenerContainer #3719

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

Merged
merged 1 commit into from
Feb 8, 2022
Merged

GH-3716: Fix wait for init redisMessageListenerContainer #3719

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions ...ion-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ protected boolean removeEldestEntry(Entry<String, RedisLock> eldest) {
private boolean executorExplicitlySet;

private volatile boolean unlinkAvailable = true;
private volatile boolean isRunningRedisMessageListenerContainer = false;

/**
* Constructs a lock registry with the default (60 second) lock expiration.
Expand Down Expand Up @@ -364,9 +365,9 @@ private boolean subscribeLock(long time) throws ExecutionException, InterruptedE
return true;
}

if (!RedisLockRegistry.this.redisMessageListenerContainer.isRunning()) {
RedisLockRegistry.this.redisMessageListenerContainer.afterPropertiesSet();
RedisLockRegistry.this.redisMessageListenerContainer.start();
if (!(RedisLockRegistry.this.isRunningRedisMessageListenerContainer
&& RedisLockRegistry.this.redisMessageListenerContainer.isRunning())) {
Copy link
Member

Choose a reason for hiding this comment

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

I think just one our isRunningRedisMessageListenerContainer is enough.
The problem is really that RedisMessageListenerContainer sets running = true; in the beginning of start().
Better to have it in the end, when subscription is ready.
But still we need that synchronized (RedisLockRegistry.this.redisMessageListenerContainer) anyway to block any concurrent calls for the start().

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is really that RedisMessageListenerContainer sets running = true; in the beginning of start().

I agree with what you said.
However, it was thought that it would be difficult to change that part in spring-integration, so I modified the code to handle it.

The reason to check both is because there is a code that sets running = false;
(isRunningRedisMessageListenerContainer and redisMessageListenerContainer.isRunning())
when an error occurs in redisMessageListenerContainer.start(), but will be not threw Exception.
(https://github.com/spring-projects/spring-data-redis/blob/main/src/main/java/org/springframework/data/redis/listener/RedisMessageListenerContainer.java#L222)

There is almost no failure, but if it fails, it looks like we will have to run start() again, so I'm checking both.

runRedisMessageListenerContainer();
}
while (time == -1 || expiredTime >= System.currentTimeMillis()) {
try {
Expand Down Expand Up @@ -523,6 +524,16 @@ private RedisLockRegistry getOuterType() {
return RedisLockRegistry.this;
}

private void runRedisMessageListenerContainer() {
synchronized (RedisLockRegistry.this.redisMessageListenerContainer) {
if (!(RedisLockRegistry.this.isRunningRedisMessageListenerContainer
&& RedisLockRegistry.this.redisMessageListenerContainer.isRunning())) {
RedisLockRegistry.this.redisMessageListenerContainer.afterPropertiesSet();
RedisLockRegistry.this.redisMessageListenerContainer.start();
RedisLockRegistry.this.isRunningRedisMessageListenerContainer = true;
}
}
}
}

private static final class RedisUnLockNotifyMessageListener implements MessageListener {
Expand Down