-
Notifications
You must be signed in to change notification settings - Fork 26.3k
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
[Dubbo-5987] Fix lock in DubboProtocol#getSharedClient #6018
Conversation
57e7f59
to
a5ec1d6
Compare
@tangcent hi, thanks for your contribution. Please reslove conficts with the latest master branch. |
@AlbumenJ done |
I think this PR is related with #6102 and #6793. In latest version of master branch, there use a lock object existed permanently to avoid concurrency issues. I am wonder if the conection key will change for each request ? If no, shall we still use the same lock object for the same connect key even when the connect key changed this will cause some memory leak. Compare with the little momory leak, reduce lock object initialization and space recycling behavior maybe more significant ? Also, using @tangcent How do you think? |
vs #6793: reduce the memory usage |
(cherry picked from commit 0f5040b) # Conflicts: # dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java
Fix lock in DubboProtocol#getSharedClient (apache#6018) See merge request framework/dubbo!63
What is the purpose of the change
Brief changelog
The situation which there is a problem:
When thread A finishes executing locks.putIfAbsent(key, new Object()),
another thread B which holding the lock executes locks.remove(key),
and then thread A executes synchronized (locks.get(key)).
Attempt get lock by computeIfAbsent like this:
1.When thread A finishes executing
synchronized (locks.computeIfAbsent(key, (k) -> new Object()))
create the lockLockA
And synchronized ,2.Another thread B synchronized with
LockA
,3.Thread A executes
locks.remove(key)
.4.Thread B hold the
LockA
And synchronized.5.Thread C executing
synchronized (locks.computeIfAbsent(key, (k) -> new Object()))
create the lockLockC
And synchronized .-It seems difficult to remove a lock without any threads waiting for it exactly.
Verifying this change
Run test cases.
Follow this checklist to help us incorporate your contribution quickly and easily:
[Dubbo-XXX] Fix UnknownException when host config not exist #XXX
. Each commit in the pull request should have a meaningful subject line and body.mvn clean install -DskipTests=false
&mvn clean test-compile failsafe:integration-test
to make sure unit-test and integration-test pass.