codeconsole opened a new pull request, #15954: URL: https://github.com/apache/grails-core/pull/15954
Fixes #14510 on 8.0.x. Identical to the 7.1.x port #15953 (this class is byte-identical on 7.1.x through 8.0.x — the cherry-pick applied with zero conflicts); see also the original 7.0.x fix #15952. ### The bug `withoutTimestamps`, `withoutDateCreated` and `withoutLastUpdated` disabled timestamping by mutating the listener-wide entity maps (capturing each entry and setting it to `Optional.empty()`, then restoring). That state is JVM-global, so overlapping windows on concurrent threads corrupt each other: 1. Thread A enters: captures ON, sets OFF 2. Thread B enters: captures OFF, sets OFF 3. Thread A exits: restores ON 4. Thread B's save now runs **with** timestamping — its historical `dateCreated`/`lastUpdated` values are silently clobbered Reproduction: https://github.com/codeconsole/grails-autotimestamp-bug A second facet: there was no `try`/`finally`, so an exception inside the closure left timestamping **permanently disabled** for the whole JVM. ### The fix The shared metadata maps are no longer mutated. Suppression is tracked per thread: the property-name getters consult a `ThreadLocal` holding an all-entities nesting depth plus the set of entity names disabled by per-class scopes. Each scope only re-enables the names it added, so nested and overlapping scopes restore correctly; `try`/`finally` guarantees restoration when the closure throws; the `ThreadLocal` is removed when empty so nothing is retained on pooled threads. Public API, protected fields and single-thread semantics are unchanged (binary and source compatible). The `CreatedBy`/`UpdatedBy` auditing maps are untouched — no `withoutXxx` API covers them. ### Merge coordination Once this and #15953 are both merged, the routine 7.1.x → 7.2.x → 8.0.x cascade merges see identical content on both sides and resolve cleanly. The only conflicted hop is 7.0.x → 7.1.x after #15952, which resolves as keep-7.1.x's-listener. ### Behavioral note Disabling is now scoped to the calling thread. Threads spawned *inside* the closure are no longer affected — under the old global behavior they were (by accident). Code relying on that must call `withoutTimestamps` on each thread. The auto-timestamping guide now documents the thread scoping. Worth a mention in the release notes. ### Tests The spec exercises the listener through the public `beforeInsert`/`beforeUpdate` API, with tests that reproduce the bug (verified red on the unfixed listener on 7.0.x and 7.1.x, whose suppression code is identical to 8.0.x's): * cross-thread isolation (the main #14510 repro) * overlapping windows on different threads restoring independently (the interleaving above) * nested and overlapping same-class scopes restoring the enclosing scope * restoration when the closure throws, including a partial failure while registering a class list * an 8-thread contention stress test All 248 tests in `grails-datamapping-core` pass on the 8.0.x toolchain (JDK 25/Groovy 5), as do the timestamp specs in `grails-datamapping-core-test`, `grails-data-hibernate5-core` and `grails-test-suite-uber`; Checkstyle and CodeNarc are clean. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
