[g0w6y]

Fixes WW-5826

ScopeInterceptor.locks is a static IdentityHashMap shared by all requests. lock()/unlock() mutate it inside synchronized (o), but o is the session map, which Dispatcher creates per request, so concurrent requests never share that monitor and the map is modified unsynchronized. It is now Collections.synchronizedMap(...), which also matches the synchronized (locks) already used by clearLocks().

Beyond the corrupted size counter, this is a slow leak under concurrent load: every entry left behind after unlock() pins a SessionMap (and through it the request and session) plus the whole ActionInvocation, for any application using the opt-in scope interceptor.

ScopeInterceptorTest#testConcurrentLockAndUnlockLeavesNoEntries runs 8 threads x 500 lock/unlock pairs on distinct objects and asserts the table is empty afterwards (via a package-private lockCount() next to clearLocks(), not reflection). Without the change it failed in every run I made; with the change it passed 5/5.

The test is statistical: it can pass without the fix on a fast machine, but it never fails with the fix.

Not changed: what ScopeInterceptor locks on. Not claiming a security issue or any cross-request data exposure; only the corrupted map size and the resulting retention were measured. The same kind of change for the XSLT plugin's BeanAdapter cache is a separate PR.

[lukaszlenart]

Thanks, the analysis holds: Dispatcher wraps the session in a new SessionMap per request (Dispatcher.java:902), so synchronized (o) never excludes two concurrent requests and the static map is mutated without a shared monitor. The one-line fix is correct and does not interfere with the o.wait()/o.notify() logic, since the map's own lock is only held inside get/put/remove.

Worth saying what the practical effect is, because the PR describes only the size counter: every entry left behind after unlock() pins a SessionMap (and through it the request and session) plus the whole ActionInvocation. So this is a slow leak under concurrent load for applications that use scope, not just a cosmetic counter. That is the reason to merge it.

Two requests before merging:

1. The test is too heavy for what it guards. 8 threads × 40,000 iterations on every core build, for a one-line change, and it never fails once the fix is in. Please cut the iterations to something small (a few hundred per thread is enough to keep it as a regression smoke test) and drop the reflection: the test is in the same package, so a package-private static int lockCount() next to clearLocks() avoids reaching into a private field.
2. The affected-versions note in WW-5826 is inaccurate. The 6.12.0 tag has the same pattern (locks is a plain IdentityHashMap, lock()/unlock() synchronize on o, only clearLocks() synchronizes on locks), so 6.x is not different from 7.x here. Please correct the ticket; which versions get the fix is decided on the ticket.

Scope note: please keep the PR to this. Changing what the interceptor locks on would be a behaviour change and is out of scope here.

[g0w6y]

@lukaszlenart Ready For Review !!