fix: uses ThreadLocal to prevent concurrent modification on ScopeManager

[filipelautert]

Impact

• Bug fix (non-breaking change which fixes expected existing functionality)
• Enhancement/New feature (adds functionality without impacting existing logic)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Issues #5426 #2433 #4173 #4314 presents the same error: "Cannot end scope bsbacsmwja when currently at scope root" .
The issue was narrowed down to Scope class handling of ScopeManager:

1. Thread "A" starts and creates a root scope. By doing so, Scope.getCurrentScope() returns "root"
2. Thread "A" starts a child scope "ABC". Now, Scope.getCurrentScope() returns "ABC", and parent is root (parent is saved in a non-static variable, while currentScope is static)
3. Thread "A" is suspended and Thread "B" starts
4. Thread "B" starts. By doing so, Scope.getCurrentScope() returns "ABC" (hey! that's already wrong)
5. Thread "B" starts a child scope "DEF". Now, Scope.getCurrentScope() returns "DEF" and parent is "ABC"
6. Thread "B" is suspended, "A" resumes
7. Thread "A" does it's job and ends the scope. The last step of exit is restoring the former Scope, so thread "A" sets currentScope to root (remember, parent is local and current is static) .
8. Thread "B" resumes - now working on curentScope "root" . It does it work and tries to finish. It tries to compare scopeId that was saved in a local variable, but then "B" gets message "Cannot end scope DEF (local variable) when currently at scope root (statically changed) " - RuntimeException

We prevent this issue by using a ThreadLocal object that ensures that each thread will have it's own ScopeManager , so "A" won't break "B" as it cannot mess with it's ScopeManager.

This issue has always been around, but seems it started to show up more as child scopes are being used for blocking tasks (that allows threads switching) such as database execution in ChangelogJdbcMdcListener . In the example above, A and B were suspended waiting for a database query to run.

[StevenMassaro]

Nice job identifying the root cause here.

Is it possible to write a unit test for this? Seems like the steps you provided in the description should be pretty straightforward to write a test for.

[rberezen]

functional test failure is expected and will be fixed with merging new s3 extension

[mches]

@filipelautert Nice fix. Would you suppose this makes ThreadLocalScopeManager effectively obsolete? Perhaps it should be deprecated?

[filipelautert]

@mches makes sense as now we are always using it, and in a simpler way.

[filipelautert]

@mches #5780 , thanks for the reminder!