Skip to content

Conversation

PiotrDuz
Copy link

@PiotrDuz PiotrDuz commented Oct 6, 2025

Closes #10446

Makes stopRenew() method thread safe.

Signed-off-by: PiotrDuz 39244523+PiotrDuz@users.noreply.github.com

Closes spring-projects#10446

Signed-off-by: PiotrDuz <39244523+PiotrDuz@users.noreply.github.com>

Signed-off-by: PiotrDuz <39244523+PiotrDuz@users.noreply.github.com>
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Signed-off-by: PiotrDuz <39244523+PiotrDuz@users.noreply.github.com>

Please, consider to use your legal name and real email.
For DCO requirements.

@PiotrDuz
Copy link
Author

PiotrDuz commented Oct 9, 2025

Hey, So I wouldn't like to post my personal data open in the web.
The name is a shortcut of my real name, so there is some association if it is really required :)

And all the details match ym account , also I sign it from the same account that authored the commit - then there is little room to claim that author has not agreed on the terms. Would it be ok ? :>

@artembilan
Copy link
Member

OK, @PiotrDuz ,

Got an answer from our management.
So, as long as the email is valid @users.noreply.github.com, we even don't need your real name.
If legal concern arises, we would reach your through GitHub support. 😄

Now I can review and merge your PR.

Thank you for your patience!

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

And feel free to add your name to the @author list of the affected class.

Thank you!

return res;
}

/* This method can be called by more than 1 thread, thus we need to make sure that it is safe*/
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this comment here.
Because it is obvious from the method and property scope that it is a bug with concurrent access.
Therefore, we follow "local variable extraction" pattern everywhere in similar situations.

Please, remove this comment altogether.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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


/home/runner/work/spring-integration/spring-integration/spring-integration-redis/src/test/java/org/springframework/integration/redis/outbound/RedisQueueOutboundChannelAdapterTests.java:134: error: cannot find symbol
		handler.setSerializer(new Jackson3JsonRedisSerializer<Object>(Object.class));
		                          ^
  symbol:   class Jackson3JsonRedisSerializer
  location: class RedisQueueOutboundChannelAdapterTests

Please, consider to rebase your branch to the latest upstream/main.
We have the fix for that Spring Data change in already.

@artembilan
Copy link
Member

Any updates, please?
We have release on next Tuesday, so would be great to make this in.
If you cannot update, no worries: we fix it at last moment before release.
Thanks

Closes spring-projects#10446
+ remove dunnecassary comment

Signed-off-by: PiotrDuz <39244523+PiotrDuz@users.noreply.github.com>
+ added author

Signed-off-by: PiotrDuz <39244523+PiotrDuz@users.noreply.github.com>
@PiotrDuz
Copy link
Author

Hey @artembilan , sorry for the delay.
I have removed the comment and rebased on the newest upstream main branch.
Regards!

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.

RedisLockRegistry stopRenew not thread safe

2 participants