RFR: Tsan oop map table hash fix

Jiangli Zhou jiangli at openjdk.org
Thu Aug 15 00:11:17 UTC 2024


Additional testing on JDK 21 with tsan found some crashes due to using `oop` `identity_hash` for computing the TsanOopMap entry hash. The crashes manifest as following two types:

1) `guarantee(moves_this_cycle) failed: "Impossible to reconcile GC"`

2) `ThreadSanitizer: CHECK failed: tsan_sync.cpp:261 "((*dst_meta)) == ((0))"`

With `-Xlog:tsan=trace` output, I found the same `oop` is added to the map twice in some cases. The second time when an `oop` is added occurs during `InterpreterMacroAssembler::lock_object` path. Apparently a new identity hash is computed for the `oop` with a previous computed identity hash. That causes the `oop` being added again. Later when notifying tsan if the `oop` is moved, it triggers the above crashes depending on the memory layout.

The fix is to compute `primitive_hash` using the oop address instead of `identity_hash`. During GC, `TsanOopMapTable::collect_moved_objects_and_notify_freed` unlinks an entry from the map if the enclosing `oop` is moved. The moved entries are collected into an array and  are inserted back to the map after iterating the map (within a worker thread). That's needed since the entry hash would be different after an `oop` is moved. The operations are protected by the `TsanOopMap_lock`.

Since `primitive_hash` is now used, I also changed `TsanOopMapTableKey::get_hash` and `TsanOopMapTableKey::equals` to use `entry.object_no_keepalive()` instead of `entry._obj`. If `entry._obj` is outdated, it could compute incorrect hash value for the current entry. So it should use the updated oop address instead. Running tsan jtreg tests on slowdebug binary run into new failures without the change.

**Testing**
Tsan jtreg tests on linux-x64:
`$ bash configure --with-boot-jdk=/usr/local/google/home/jianglizhou/openjdk/jdk-21.0.1 --with-debug-level=release --disable-warnings-as-errors --with-jtreg=/usr/local/google/home/jianglizhou/github/jtreg/build/images/jtreg --with-stdc++lib=static --disable-precompiled-headers --enable-unlimited-crypto --with-native-debug-symbols=internal --with-default-make-target=jdk-image --disable-warnings-as-errors --with-toolchain-type=clang`
==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/hotspot/jtreg/tsan                        79    79     0     0   
==============================
TEST SUCCESS

-------------

Commit messages:
 - - Remove obj->fast_no_hash_check() case from TsanOopMapTable::add_entry. We don't use oop
 - Add TsanOopMapImpl::MovedEntry. Also save the oop size for a moved oop and avoid calling oop size() when adding a moved
 - Update comments.
 - Remove moved_entry_sizes array.
 - Change to compute primitive_hash using the oop address instead of oop identity_hash for an entry's hash.

Changes: https://git.openjdk.org/tsan/pull/23/files
  Webrev: https://webrevs.openjdk.org/?repo=tsan&pr=23&range=00
  Stats: 65 lines in 3 files changed: 53 ins; 4 del; 8 mod
  Patch: https://git.openjdk.org/tsan/pull/23.diff
  Fetch: git fetch https://git.openjdk.org/tsan.git pull/23/head:pull/23

PR: https://git.openjdk.org/tsan/pull/23


More information about the tsan-dev mailing list