Change DubboShutdownHook to singleton and process level

[Chenjp]

When we register DubboShutdownHook, actions-on-exit are expected. So we should not disable the Hook.

What is the purpose of the change

Change DubboShutdownHook to singleton, and make it as process level Hook. Fix #14429.

Brief changelog

As declared, ShutdownHook should perform process-exit ops, familiar with dubbo engine. Move DubboShutdownHook#addShutdownHook back to DubboBootstrap.

Update DubboShutdownHookTest to add more internal check and improve code coverage.

Verifying this change

Checklist

• [x] Make sure there is a GitHub_issue field 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.
• [ ] 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.
• [ ] Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
• [ ] 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 sample in dubbo samples project.
• [ ] Add some description to dubbo-website project if you are requesting to add a feature.
• [ ] GitHub Actions works fine on your own branch.
• [ ] If this contribution is large, please follow the Software Donation Guide.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 70.12%. Comparing base (3e53e32) to head (9d32562). Report is 82 commits behind head on 3.2.

Additional details and impacted files

@@            Coverage Diff             @@
##              3.2   #14432      +/-   ##
==========================================
+ Coverage   70.11%   70.12%   +0.01%     
==========================================
  Files        1607     1608       +1     
  Lines       70192    70206      +14     
  Branches    10116    10116              
==========================================
+ Hits        49213    49231      +18     
- Misses      16324    16327       +3     
+ Partials     4655     4648       -7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[AlbumenJ]

Ignore this exception would be better

[Chenjp]

We should remove the shutdown hook when Dubbo Application has been shutdowned by API.

@AlbumenJ I am confused by DubboShutdownHook. Per constructor, dubboShutdownHook is ApplicationModel level. But following lines shows that its may impact other application models of same fwk model.

        // send readonly for shutdown hook
        List<GracefulShutdown> gracefulShutdowns =
                GracefulShutdown.getGracefulShutdowns(applicationModel.getFrameworkModel());
        for (GracefulShutdown gracefulShutdown : gracefulShutdowns) {
            gracefulShutdown.readonly();
        }

// if all FrameworkModels are destroyed, clean global static resources, shutdown dubbo completely

According the above comment, dubbo server supports more than one FrameworkModel. TODO:

1. move ApplicationModel (and sub-models) cleanup functions back to ApplicationModel#onDestroy
2. Fwk model level cleanup - to FrameworkModel#onDestroy
3. Runtime hook on process exit, shutdown dubbo engine gracefully: DubboShutdownHook supposed to do.

[AlbumenJ]

• move ApplicationModel (and sub-models) cleanup functions back to ApplicationModel#onDestroy
• Fwk model level cleanup - to FrameworkModel#onDestroy
• Runtime hook on process exit, shutdown dubbo engine gracefully: DubboShutdownHook supposed to do.

Agree

[Chenjp]

After my commits, following stack traces shows that recursive destroy occur (framework->application->framework). @AlbumenJ may code refactoring is expected to match the productive-level design.

We already define framework-application-module hierarchical relationship clearly in ScopeModel#internalIdcomment section. And if we follow the relationship rule strictly, then XxxModel#destroyed.compareAndSet should have never been used.

Request.setVersion(String) line: 95	
HeaderExchangeServer.sendChannelReadOnlyEvent() line: 134	
HeaderExchangeServer.close(int) line: 111	
DubboProtocol.destroy() line: 610	
ProtocolSecurityWrapper.destroy() line: 117	
ProtocolListenerWrapper.destroy() line: 99	
ProtocolFilterWrapper.destroy() line: 80	
ProtocolSerializationWrapper.destroy() line: 60	
InvokerCountWrapper.destroy() line: 55	
FrameworkModelCleaner.destroyProtocols(FrameworkModel) line: 68	
FrameworkModelCleaner.destroyFrameworkResources(FrameworkModel) line: 55	
FrameworkModelCleaner.onDestroy(FrameworkModel) line: 47	
FrameworkModelCleaner.onDestroy(ScopeModel) line: 1	
FrameworkModel(ScopeModel).notifyProtocolDestroy() line: 155	
FrameworkModel.tryDestroyProtocols() line: 280	
ApplicationModel.onDestroy() line: 159	
ApplicationModel(ScopeModel).destroy() line: 122	
FrameworkModel.onDestroy() line: 129	
FrameworkModel(ScopeModel).destroy() line: 122	
FrameworkModel.destroyAll() line: 210	
DubboShutdownHook.run() line: 128

[Chenjp]

• move ApplicationModel (and sub-models) cleanup functions back to ApplicationModel#onDestroy
• Fwk model level cleanup - to FrameworkModel#onDestroy
• Runtime hook on process exit, shutdown dubbo engine gracefully: DubboShutdownHook supposed to do.

Agree

Done. @AlbumenJ PTAL

[sonarqubecloud[bot]]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[Chenjp]

no more news.