fix: locking on FeatureFlags instead of VaadinContext (#13962)

[MarcinVaadin]

Description

Potential solutions (I did not really think these through):

Lock on vaadin context in getCachedIndexHtmlDocument to avoid locking in another order. Seems quite fragile and there might be other similar cases here and there.
Use the same object for locking in all cases. Can't see why we really need to synchronize on different ones for these operations

Only lock on FeatureFlags when checking for wrapper and initializing

Fixes #13962

Type of change

• Bugfix
• Feature

Checklist

• I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
• I have added a description following the guideline.
• The issue is created in the corresponding repository and I have referenced it.
• I have added tests to ensure my change is effective and works as intended.
• New and existing tests are passing locally with my change.
• I have performed self-review and corrected misspellings.

Additional for Feature type of change

• Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

[github-actions]

Unit Test Results

   955 files  ±  0     955 suites  ±0   58m 2s ⏱️ + 4m 6s
6 082 tests ±  0  6 028 ✔️ ±  0  54 💤 ±0  0 ❌ ±0 
6 348 runs  +11  6 286 ✔️ +11  62 💤 ±0  0 ❌ ±0 

Results for commit 9111842. ± Comparison against base commit dc00ef1.

♻️ This comment has been updated with latest results.

[Artur-]

Is VaadinServletContext a singleton? Otherwise locking will have no effect

[MarcinVaadin]

Is VaadinServletContext a singleton? Otherwise locking will have no effect

@Artur- indeed, so why to lock on VaadinContext in FeatureFlags if they call setAttributes which already locks on ServletContext?

[Artur-]

The locking in FeatureFlags looks wrong. It wants to lock to avoid creating FeatureFlagsWrapper multiple times. The normal pattern for that would be something like

FeatureFlagsWrapper attribute = context.getAttribute(FeatureFlagsWrapper.class);
if (attribute == null) {
  synchronized (FeatureFlags.class) {
    attribute = context.getAttribute(FeatureFlagsWrapper.class);
    if (attribute == null) {
        // create it
    }
  }
}
return attribute.getFeatureFlags();

There is no need to lock on anything external as only FeatureFlags is creating the wrapper.

The simpler version which locks every time, and might be good enough in FeatureFlags would be

synchronized (FeatureFlags.class) {
  FeatureFlagsWrapper attribute = context.getAttribute(FeatureFlagsWrapper.class);
  if (attribute == null) {
      // create it
  }
}
return attribute.getFeatureFlags();

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[Artur-]

Looks good to me but the fix does not correspond at all with the summary or description