Put the lock acquisition in front of try by ZhichX · Pull Request #1944 · apache/dubbo

ZhichX · 2018-06-14T13:54:49Z

What is the purpose of the change

Put lock.lock() in try{...}, when lock.lock() reports an error, it will also enter the finally release lock. According to the Lock.unlock() document, an unchecked exception will be thrown when the non-lock holding thread calls this method:

Brief changelog

Put the lock acquisition in front of try

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a GITHUB_issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
Run mvn clean install -DskipTests & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
If this contribution is large, please follow the Software Donation Guide.

# Conflicts: # dubbo-filter/dubbo-filter-cache/src/test/java/com/alibaba/dubbo/cache/filter/CacheFilterTest.java

carryxyh

u should rebase your branch onto remote master first
And also, there is a pr which is rename the repackage to org.apache. Please resend a PR after the repackage PR

codecov-io · 2018-06-14T14:24:57Z

Codecov Report

Merging #1944 into master will increase coverage by 0.09%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master   #1944      +/-   ##
===========================================
+ Coverage     50.31%   50.4%   +0.09%     
- Complexity     4736    4740       +4     
===========================================
  Files           518     518              
  Lines         25421   25424       +3     
  Branches       4546    4560      +14     
===========================================
+ Hits          12790   12815      +25     
+ Misses        10670   10654      -16     
+ Partials       1961    1955       -6

Impacted Files	Coverage Δ	Complexity Δ
...he/dubbo/remoting/transport/netty/NettyClient.java	`72.88% <0%> (-1.26%)`	`12% <0%> (ø)`
...e/dubbo/remoting/transport/netty4/NettyClient.java	`63.33% <0%> (-1.08%)`	`11% <0%> (ø)`
...ache/dubbo/remoting/transport/mina/MinaClient.java	`58.46% <0%> (-0.92%)`	`9% <0%> (ø)`
.../java/org/apache/dubbo/config/ReferenceConfig.java	`47.68% <0%> (+0.71%)`	`38% <0%> (ø)`	⬇️
.../dubbo/rpc/protocol/dubbo/DecodeableRpcResult.java	`45.31% <0%> (+1.56%)`	`8% <0%> (ø)`	⬇️
...he/dubbo/registry/multicast/MulticastRegistry.java	`57.74% <0%> (+1.67%)`	`41% <0%> (+3%)`	⬆️
...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java	`89.71% <0%> (+1.86%)`	`15% <0%> (ø)`	⬇️
...in/java/org/apache/dubbo/common/utils/IOUtils.java	`86.95% <0%> (+2.17%)`	`16% <0%> (+1%)`	⬆️
...va/org/apache/dubbo/remoting/exchange/Request.java	`85.71% <0%> (+2.38%)`	`20% <0%> (ø)`	⬇️
...bo/rpc/protocol/dubbo/telnet/LogTelnetHandler.java	`71.42% <0%> (+2.85%)`	`3% <0%> (ø)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e92a553...d84ffde. Read the comment docs.

diecui1202 · 2018-06-15T01:47:02Z

Hi, @ZhichX As now we are working on the rename package to org.apache, so I suggest you could resend a new PR after that is merged.

ZhichX · 2018-06-15T01:51:25Z

ok， I'll resend a new PR

diecui1202 · 2018-06-15T01:58:04Z

Appreciate for that.

# Conflicts: # dubbo-common/src/main/java/com/alibaba/dubbo/common/utils/LRUCache.java # dubbo-config/dubbo-config-api/src/main/java/com/alibaba/dubbo/config/DubboShutdownHook.java # dubbo-container/dubbo-container-api/src/main/java/com/alibaba/dubbo/container/Main.java # dubbo-filter/dubbo-filter-cache/src/main/resources/META-INF/dubbo/internal/com.alibaba.dubbo.cache.CacheFactory # dubbo-filter/dubbo-filter-cache/src/test/java/com/alibaba/dubbo/cache/filter/CacheFilterTest.java # dubbo-rpc/dubbo-rpc-dubbo/src/test/java/com/alibaba/dubbo/rpc/protocol/dubbo/ExplicitCallbackTest.java

谢志春 zhichun.xie added 22 commits April 26, 2018 23:56

ExpiryCache - With the characteristic of expiration time

8caffd4

ExpiryCache - With the characteristic of expiration time

f08ccb7

fix 1 Checkstyle violation

12d7b9f

Merge remote-tracking branch 'alibaba/master'

648975e

# Conflicts: # dubbo-filter/dubbo-filter-cache/src/test/java/com/alibaba/dubbo/cache/filter/CacheFilterTest.java

add destroy method for cache to release resources

037d835

add destroy method for cache to release resources

30e3755

fix a Checkstyle violation

70a5dd4

fix a Checkstyle violation

d4ef025

add expiryCacheFactoryTest

038d64f

Merge remote-tracking branch 'alibaba/master'

70bef98

delete unused comments code

14ba3a3

Merge remote-tracking branch 'alibaba/master'

d12bb9e

fix ShutdownHook Close a non-SPI cache

097cf48

delete test code

4ed07a0

delete ExpiringMap's deamon thread release and rename Expiry to Expiring

3f44d29

rename package expiry to expiring

c56e7da

remove lock for ExpiryObject

bf593fd

move unused import

33a1d5e

rollback my change made against CacheFilterTest

115bf7d

add expiring into CacheFilterTest

4af54dc

Put the lock acquisition in front of try

f314cc8

fix a Checkstyle violation (import *)

466c6ac

carryxyh reviewed Jun 14, 2018

View reviewed changes

谢志春 zhichun.xie added 3 commits June 15, 2018 15:58

Put the lock acquisition in front of try

5e3de83

remove some unused package after repackage

d84ffde

ulinkwo mentioned this pull request Jun 15, 2018

[Dubbo-1927]: Enhance IDE support for custom spring namespace elements (dubbo) #1953

Merged

6 tasks

ZhichX mentioned this pull request Jun 20, 2018

Optimize the use of lock.lock() #1963

Closed

ZhichX closed this Jun 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Put the lock acquisition in front of try#1944

Put the lock acquisition in front of try#1944
ZhichX wants to merge 25 commits intoapache:masterfrom
ZhichX:master

ZhichX commented Jun 14, 2018

Uh oh!

carryxyh left a comment •

edited

Loading

Uh oh!

codecov-io commented Jun 14, 2018 •

edited

Loading

Uh oh!

diecui1202 commented Jun 15, 2018

Uh oh!

ZhichX commented Jun 15, 2018

Uh oh!

diecui1202 commented Jun 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ZhichX commented Jun 14, 2018

What is the purpose of the change

Brief changelog

Verifying this change

Uh oh!

carryxyh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jun 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

diecui1202 commented Jun 15, 2018

Uh oh!

ZhichX commented Jun 15, 2018

Uh oh!

diecui1202 commented Jun 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

carryxyh left a comment •

edited

Loading

codecov-io commented Jun 14, 2018 •

edited

Loading