Resolving the Thread-Safety Problem in registerReferenceKeyAndBeanName Using an Overloaded computeIfAbsent

[juzzia]

org.apache.dubbo.config.spring.reference.ReferenceBeanManager.java

    public void registerReferenceKeyAndBeanName(String referenceKey, String referenceBeanNameOrAlias) {
        List<String> list =
                ConcurrentHashMapUtils.computeIfAbsent(referenceKeyMap, referenceKey, (key) -> new ArrayList<>());
        if (!list.contains(referenceBeanNameOrAlias)) {
            list.add(referenceBeanNameOrAlias);
            // register bean name as alias
            referenceAliasMap.put(referenceBeanNameOrAlias, list.get(0));
        }
    }

The ConcurrentHashMapUtils.computeIfAbsent method ensures the creation of a new list in a thread-safe manner when no list exists for the given referenceKey. However, the subsequent operations if (!list.contains(referenceBeanNameOrAlias)) and list.add(referenceBeanNameOrAlias) are not atomic. This situation can lead to thread-safety issues where multiple threads might simultaneously pass the contains check and then all perform the add operation, resulting in duplicate referenceBeanNameOrAlias entries within the list.

To resolve this issue, I added an overloaded version of the computeIfAbsent method that allows for thread-safe operations on the value through the use of a Consumer functional interface. Additionally, I provided concurrent unit tests:

• ConcurrentHashMapUtilsTest#threadSafetyOperatorForJava8Test
• ConcurrentHashMapUtilsTest#threadSafetyOperatorForJava17Test
  These tests confirm the correctness of the implementation.

The profile defined here cannot run the spotless plugin in JDK 8 because the version of the Spotless plugin is too high. Therefore, I've overridden the version of the Spotless plugin in the jdk8 - jdk11 profile to make it compatible with the lower - version environment. spotless plugin version 2.22.0 can work properly in JDK versions ranging from 8 to 11.

    <profile>
      <id>jdk9-jdk11-spotless</id>
      <activation>
        <jdk>[1.8, 11)</jdk>
      </activation>
      <properties>
        <palantirJavaFormat.version>1.1.0</palantirJavaFormat.version>
      </properties>
    </profile>

    <properties>
      <spotless-maven-plugin.lower.version>2.22.0</spotless-maven-plugin.lower.version>
    </properties>
    <profile>
      <id>jdk8-jdk11-spotless</id>
      <activation>
        <jdk>[1.8, 11)</jdk>
      </activation>
      <properties>
        <palantirJavaFormat.version>1.1.0</palantirJavaFormat.version>
        <spotless-maven-plugin.version>${spotless-maven-plugin.lower.version}</spotless-maven-plugin.version>
      </properties>
    </profile>

Use a lower version of the spotless-maven-plugin dependency to override the version defined by spotless-maven-plugin.version so as to support JDK 1.8.

[AlbumenJ]

[juzzia]

Thank you very much for the screenshot 👍. Due to continuous errors when downloading local Maven dependencies, the project still cannot run mvn spotless:apply. Therefore, I have been trying to push my code to check if my code formatting passes the checks 😭. I will use your screenshot as a reference and try to reformat my code again.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.60%. Comparing base (d3b1e1f) to head (c446ddb).
⚠️ Report is 171 commits behind head on 3.3.

Files with missing lines	Patch %	Lines
...che/dubbo/common/utils/ConcurrentHashMapUtils.java	90.90%	0 Missing and 1 partial ⚠️
.../config/spring/reference/ReferenceBeanManager.java	90.00%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (d3b1e1f) and HEAD (c446ddb). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (d3b1e1f)	HEAD (c446ddb)
unit-tests	1	0

Additional details and impacted files

@@              Coverage Diff              @@
##                3.3   #15219       +/-   ##
=============================================
- Coverage     60.79%   35.60%   -25.19%     
+ Complexity    10892    10881       -11     
=============================================
  Files          1885     1885               
  Lines         86082    86091        +9     
  Branches      12895    12895               
=============================================
- Hits          52331    30655    -21676     
- Misses        28303    50994    +22691     
+ Partials       5448     4442     -1006     

Flag	Coverage Δ
integration-tests	33.15% <85.71%> (+<0.01%)	⬆️
samples-tests	29.19% <90.47%> (+<0.01%)	⬆️
unit-tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[juzzia]

@AlbumenJ I'm not sure why the workflow execution timed out since my code changes should not affect the test runs of the AbstractDynamicConfigurationTest class. I suspect it might be due to dependent services. Could you please rerun the PR action?

[zrlw]

@AlbumenJ I'm not sure why the workflow execution timed out since my code changes should not affect the test runs of the AbstractDynamicConfigurationTest class. I suspect it might be due to dependent services. Could you please rerun the PR action?

You can trigger it on your own Github Repo Actions like this,

[zrlw]

since the author doesn't respond for a long time, i submit a new pr #15258