Skip to content

Make AutoTimestampEventListener temporary disabling thread-safe (8.0.x)#15954

Open
codeconsole wants to merge 1 commit into
apache:8.0.xfrom
codeconsole:fix/autotimestamp-thread-safety-8.0.x
Open

Make AutoTimestampEventListener temporary disabling thread-safe (8.0.x)#15954
codeconsole wants to merge 1 commit into
apache:8.0.xfrom
codeconsole:fix/autotimestamp-thread-safety-8.0.x

Conversation

@codeconsole

Copy link
Copy Markdown
Contributor

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):

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.

Fixes apache#14510. The withoutTimestamps, withoutDateCreated and
withoutLastUpdated methods disabled timestamping by mutating the
listener-wide entity maps (capturing each entry and setting it to
Optional.empty()), which is JVM-global state. Overlapping windows on
concurrent threads corrupted each other: a thread leaving its window
re-enabled timestamping for every other thread still inside one, so
historical dateCreated/lastUpdated values were silently overwritten.
An exception inside the closure also left timestamping permanently
disabled because the previous state was restored without try/finally.

Suppression is now 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, and try/finally guarantees restoration when the
closure throws. The shared metadata maps are no longer mutated. The
CreatedBy/UpdatedBy auditing maps are untouched, as no withoutXxx
API covers them.

Behavioral note: disabling is now scoped to the calling thread;
threads spawned inside the closure are no longer affected. The GORM
auto-timestamping guide documents this.

The spec now exercises the listener through beforeInsert/beforeUpdate
and covers cross-thread isolation, overlapping windows, nested and
overlapping same-class scopes, exception restoration and concurrent
use.

Forward-port of the 7.0.x fix (apache#15952) to 8.0.x, identical to the
7.1.x port (apache#15953), as this class is identical on 7.1.x through 8.0.x.
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 49.5213%. Comparing base (fa25f0a) to head (c3289a8).

Files with missing lines Patch % Lines
...astore/gorm/events/AutoTimestampEventListener.java 97.3684% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.0.x     #15954        +/-   ##
==================================================
+ Coverage     49.5127%   49.5213%   +0.0086%     
- Complexity      16895      16901         +6     
==================================================
  Files            1998       1998                
  Lines           93679      93695        +16     
  Branches        16402      16403         +1     
==================================================
+ Hits            46383      46399        +16     
+ Misses          40139      40138         -1     
- Partials         7157       7158         +1     
Files with missing lines Coverage Δ
...astore/gorm/events/AutoTimestampEventListener.java 67.8392% <97.3684%> (+2.2654%) ⬆️

... and 1 file with indirect coverage changes

🚀 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.

@testlens-app

testlens-app Bot commented Jul 10, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: c3289a8
▶️ Tests: 28018 executed
⚪️ Checks: 59/59 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

GRAILS 6+ BUG: autoTimestampEventListener.without* Results in Unexpected Behavior Across All Threads

1 participant