Skip to content
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

Add tag variables to avoid repeated calls to ExporterChangeableWrapper#unexport method #7819

Merged
merged 2 commits into from
May 23, 2021

Conversation

BurningCN
Copy link
Member

@BurningCN BurningCN commented May 21, 2021

When the dubbo program is destroyed, for example, start from spring's context.stop(). DubboBootstrap#destroy will be called, and the ExporterChangeableWrapper#unexport method will be called twice in the logic.

  • The first is in destroyProtocols()->protocol.destroy()->Registry#destroy()->exporter.unexport()->ExporterChangeableWrapper#unexport
  • the second place is in unexportServices()->sc.unexport()->exporter.unexport()->DestroyableExporter#unexport->ExporterChangeableWrapper#unexport

So, I add this unexported variable to avoid repeated unexport

@codecov-commenter
Copy link

codecov-commenter commented May 21, 2021

Codecov Report

Merging #7819 (6c19340) into master (e845e16) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7819      +/-   ##
============================================
+ Coverage     59.28%   59.40%   +0.12%     
- Complexity      529      556      +27     
============================================
  Files          1077     1086       +9     
  Lines         43565    43842     +277     
  Branches       6368     6396      +28     
============================================
+ Hits          25826    26043     +217     
- Misses        14879    14913      +34     
- Partials       2860     2886      +26     
Impacted Files Coverage Δ Complexity Δ
...e/dubbo/registry/integration/RegistryProtocol.java 60.49% <100.00%> (-0.26%) 0.00 <0.00> (ø)
.../dubbo/remoting/transport/MultiMessageHandler.java 80.00% <0.00%> (-20.00%) 0.00% <0.00%> (ø%)
...bo/rpc/cluster/support/FailbackClusterInvoker.java 52.45% <0.00%> (-18.04%) 0.00% <0.00%> (ø%)
...in/java/org/apache/dubbo/common/utils/JVMUtil.java 81.13% <0.00%> (-11.33%) 0.00% <0.00%> (ø%)
...va/org/apache/dubbo/rpc/support/ProtocolUtils.java 61.53% <0.00%> (-8.03%) 0.00% <0.00%> (ø%)
.../threadpool/manager/DefaultExecutorRepository.java 80.95% <0.00%> (-4.54%) 0.00% <0.00%> (ø%)
.../rpc/protocol/dubbo/LazyConnectExchangeClient.java 49.45% <0.00%> (-4.40%) 0.00% <0.00%> (ø%)
.../apache/dubbo/remoting/transport/AbstractPeer.java 58.69% <0.00%> (-4.35%) 0.00% <0.00%> (ø%)
...ng/transport/dispatcher/WrappedChannelHandler.java 58.69% <0.00%> (-4.35%) 0.00% <0.00%> (ø%)
...n/java/org/apache/dubbo/common/utils/LRUCache.java 82.22% <0.00%> (-3.99%) 0.00% <0.00%> (ø%)
... and 80 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 e845e16...6c19340. Read the comment docs.

@@ -795,6 +797,10 @@ public void setExporter(Exporter<T> exporter) {

@Override
public void unexport() {
if (unexported) {
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to use AtomicBoolean, and use cas to set ture here to prevent concurrency problem

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, according to your suggestion, it has been adjusted to AtomicBoolean type

@AlbumenJ AlbumenJ merged commit b16fce5 into apache:master May 23, 2021
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.

3 participants