Skip to content

Conversation

kdebski85
Copy link
Contributor

fixes: #3980

Fix concurrent initialization in DefaultSftpSessionFactory

@pivotal-cla
Copy link

@kdebski85 Please sign the Contributor License Agreement!

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

See the FAQ for frequently asked questions.

@artembilan
Copy link
Member

Thank you, but I have already made the fix here: #3981.
WDYT?

@kdebski85
Copy link
Contributor Author

kdebski85 commented Jan 4, 2023

My approach might be slightly faster because:

  1. I do not use synchronized
  2. I do not use AtomicBoolean

Can I copy test from your PR?

@pivotal-cla
Copy link

@kdebski85 Thank you for signing the Contributor License Agreement!

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.

@kdebski85 ,

any chances to see a unit test to confirm that we don't have concurrency issue any more?
As we discussed before you can simply copy-paste a test I provide in my rejected PR: https://github.com/spring-projects/spring-integration/pull/3981/files#diff-ea9ddb7293f89610654c162b3b862b3b590008c3feb48e12ec2d57c69027d16eR96

Thanks

@kdebski85
Copy link
Contributor Author

kdebski85 commented Jan 5, 2023

@artembilan
I copied test from #3981.
Please review.

@kdebski85 kdebski85 requested a review from artembilan January 5, 2023 09:29
@artembilan artembilan merged commit 854e555 into spring-projects:main Jan 5, 2023
@artembilan
Copy link
Member

@kdebski85 ,

thank you for contribution and reasonable discussion; looking forward for more!

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.

Concurrent threads might use uninitialised SFTP client

5 participants