Work through thread safety issues in runtime reflection reported by TSAN

[retronym]

Here are the results of running a threaded test of runtime reflection through the experimental OpenJDK TSAN project.

https://gist.github.com/retronym/2e6520815afbf002a0893d9f5ed0aed5

I manually identified and fix a pair of problems in scala/scala#8397.

WIP for the other problems here that TSAN is revealing: retronym/scala#70

UndoLog.<init> calls perRunCaches

https://github.com/scala/scala/blob/7bf179f69703460e6d06e40db2fa799c33b63038/src/reflect/scala/reflect/internal/tpe/TypeConstraints.scala#L38

There is a separate UndoLog per thread, but we need to guard the new UndoLog in the reflection lock or maybe just remove the registration with per-run caches when !isCompilerUniverse

data race on Global.phase

WARNING: ThreadSanitizer: data race (pid=29788)
  Write of size 4 at 0x0000d6f60d68 by thread T23:
    #0 scala.reflect.internal.SymbolTable.phase_$eq(Lscala/reflect/internal/Phase;)V SymbolTable.scala:240
    #1 scala.reflect.internal.Symbols$Symbol.completeInfo()V Symbols.scala:1543
    #2 scala.reflect.internal.Symbols$Symbol.info()Lscala/reflect/internal/Types$Type; Symbols.scala:1517

  Previous read of size 4 at 0x0000d6f60d68 by thread T14 (mutexes: write M798263037216184320):
    #0 scala.reflect.internal.SymbolTable.phase()Lscala/reflect/internal/Phase; SymbolTable.scala:231
    #1 scala.reflect.internal.Symbols$Symbol.flags()J Symbols.scala:769
    #2 scala.reflect.internal.tpe.FindMembers$FindMemberBase.walkBaseClasses(JJ)Z FindMembers.scala:100
    #3 scala.reflect.internal.tpe.FindMembers$FindMemberBase.searchConcreteThenDeferred()Ljava/lang/Object; FindMembers.scala:65
    #4 scala.reflect.internal.tpe.FindMembers$FindMemberBase.apply()Ljava/lang/Object; FindMembers.scala:57
    #5 scala.reflect.internal.Types$Type.findMemberInternal$1(Lscala/reflect/internal/Names$Name;JJZ)Lscala/reflect/internal/Symbols$Symbol; Types.scala:1033
    #6 scala.reflect.internal.Types$Type.findMember(Lscala/reflect/internal/Names$Name;JJZ)Lscala/reflect/internal/Symbols$Symbol; Types.scala:1035
    #7 scala.reflect.internal.Types$Type.memberBasedOnName(Lscala/reflect/internal/Names$Name;J)Lscala/reflect/internal/Symbols$Symbol; Types.scala:667
    #8 scala.reflect.internal.Types$Type.member(Lscala/reflect/internal/Names$Name;)Lscala/reflect/internal/Symbols$Symbol; Types.scala:631
    #9 scala.reflect.internal.Mirrors$RootsBase.staticModule(Ljava/lang/String;)Lscala/reflect/internal/Symbols$ModuleSymbol; Mirrors.scala:59
    #10 scala.reflect.internal.Mirrors$RootsBase.staticModule(Ljava/lang/String;)Lscala/reflect/api/Symbols$ModuleSymbolApi; Mirrors.scala:29
    #11 scala.reflect.runtime.ThreadSafetyTest$$typecreator5$1.apply(Lscala/reflect/api/Mirror;)Lscala/reflect/api/Types$TypeApi; ThreadSafetyTest.scala:83
    #12 scala.reflect.api.TypeTags$WeakTypeTagImpl.tpe$lzycompute()Lscala/reflect/api/Types$TypeApi; TypeTags.scala:237
    #13 scala.reflect.api.TypeTags$WeakTypeTagImpl.tpe()Lscala/reflect/api/Types$TypeApi; TypeTags.scala:237
    #14 (Unknown Method) <null>
    #15 (Unknown Method) <null>
    #16 scala.reflect.api.Universe.typeOf(Lscala/reflect/api/TypeTags$TypeTag;)Lscala/reflect/api/Types$TypeApi; Universe.scala:73
    #17 scala.reflect.runtime.ThreadSafetyTest.$anonfun$test$6()Lscala/reflect/api/Types$TypeApi; ThreadSafetyTest.scala:83

This is benign because in the runtime reflection universe the assignment is a no-op (Global.ph always has the dummy value SomePhase`).

We could guard the call with isCompilerUniverse to silence this warning.

substTypeMapCache data race

SUMMARY: ThreadSanitizer: data race Types.scala:114 in scala.reflect.internal.Types$substTypeMapCache$.apply(Lscala/collection/immutable/List;Lscala/collection/immutable/List;)Lscala/reflect/internal/tpe/TypeMaps$SubstTypeMap;

Found manually and fixes in scala/scala#8397.

TypeRef.normalized data race

SUMMARY: ThreadSanitizer: data race Types.scala:2517 in scala.reflect.internal.Types$TypeRef.normalize()Lscala/reflect/internal/Types$Type;

Multiple threads update this cache of the normalized type. Called, for example, within subtyping.

We could:

• take the reflection lock in this part of the code
• mark the cache as volatile and use a CAS to update it in a lock free way that makes sure that all threads see the same result, even if we let two threads race to create the normalized type.

Data race with access to flags

  Write of size 8 at 0x0000c38e3b98 by thread T11:
    #0 scala.reflect.internal.Symbols$Symbol.completeInfo()V Symbols.scala:1538
    #1 scala.reflect.internal.Symbols$Symbol.info()Lscala/reflect/internal/Types$Type; Symbols.scala:1517
    #2 scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$7.scala$reflect$runtime$SynchronizedSymbols$SynchronizedSymbol$$super$info()Lscala/reflect/internal/Types$Type; SynchronizedSymbols.scala:202
    #3 (Unknown Method) <null>
    #4 (Unknown Method) <null>
    #5 scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$7.info()Lscala/reflect/internal/Types$Type; SynchronizedSymbols.scala:202
    #6 scala.reflect.internal.Symbols$Symbol.initialize()Lscala/reflect/internal/Symbols$Symbol; Symbols.scala:1691
    #7 (Unknown Method) <null>
    #8 (Unknown Method) <null>
    #9 scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$7.getFlag(J)J SynchronizedSymbols.scala:202
    #10 scala.reflect.internal.Symbols$Symbol.hasFlag(J)Z Symbols.scala:744
    #11 scala.reflect.internal.Definitions$DefinitionsClass.unspecializedSymbol(Lscala/reflect/internal/Symbols$Symbol;)Lscala/reflect/internal/Symbols$Symbol; Definitions.scala:664
    #12 scala.reflect.internal.Definitions$DefinitionsClass.isFunctionSymbol(Lscala/reflect/internal/Symbols$Symbol;)Z Definitions.scala:659
    #13 scala.reflect.internal.Definitions$DefinitionsClass.isFunctionTypeDirect(Lscala/reflect/internal/Types$Type;)Z Definitions.scala:730

  Previous read of size 8 at 0x0000c38e3b98 by thread T17:
    #0 scala.reflect.internal.Symbols$Symbol.rawflags()J Symbols.scala:259
    #1 scala.reflect.internal.Symbols$Symbol.getFlag(J)J Symbols.scala:741
    #2 scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$7.scala$reflect$runtime$SynchronizedSymbols$SynchronizedSymbol$$super$getFlag(J)J SynchronizedSymbols.scala:202
    #3 (Unknown Method) <null>
    #4 (Unknown Method) <null>
    #5 scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$7.getFlag(J)J SynchronizedSymbols.scala:202
    #6 scala.reflect.internal.Symbols$Symbol.hasFlag(J)Z Symbols.scala:744
    #7 (Unknown Method) <null>
    #8 (Unknown Method) <null>
    #9 scala.reflect.internal.Symbols$Symbol.hasPackageFlag()Z Symbols.scala:221
    #10 (Unknown Method) <null>
    #11 (Unknown Method) <null>
    #12 (Unknown Method) <null>
    #13 (Unknown Method) <null>
    #14 (Unknown Method) <null>
    #15 scala.reflect.internal.SymbolTable.isSameType2(Lscala/reflect/internal/Types$Type;Lscala/reflect/internal/Types$Type;)Z SymbolTable.scala:28

Supposedly safe according to:

https://github.com/scala/scala/blob/2d9e6acce8c4246e6cba5a22ff9d965ec3bd117c/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala#L119-L155

race SingleType.underlying

WARNING: ThreadSanitizer: data race (pid=32590)
  Write of size 4 at 0x0000c2b5fc1c by thread T13:
    #0 scala.reflect.internal.Types$SingleType.underlyingPeriod_$eq(I)V Types.scala:1231
    #1 (Unknown Method) <null>
    #2 (Unknown Method) <null>
    #3 scala.reflect.runtime.JavaUniverse.scala$reflect$runtime$SynchronizedTypes$$super$defineUnderlyingOfSingleType(Lscala/reflect/internal/Types$SingleType;)V JavaUniverse.scala:30
    #4 (Unknown Method) <null>
    #5 scala.reflect.runtime.SynchronizedTypes$$Lambda$502.apply$mcV$sp()V ??
    #6 (Unknown Method) <null>
    #7 (Unknown Method) <null>
    #8 (Unknown Method) <null>
    #9 scala.reflect.runtime.JavaUniverse.gilSynchronized(Lscala/Function0;)Ljava/lang/Object; JavaUniverse.scala:30
    #10 (Unknown Method) <null>
    #11 (Unknown Method) <null>
    #12 scala.reflect.runtime.JavaUniverse.defineUnderlyingOfSingleType(Lscala/reflect/internal/Types$SingleType;)V JavaUniverse.scala:30
    #13 scala.reflect.internal.Types$SingleType.underlying()Lscala/reflect/internal/Types$Type; Types.scala:1240
    #14 (Unknown Method) <null>
    #15 (Unknown Method) <null>
    #16 scala.reflect.internal.Types$SingletonType.typeSymbolDirect()Lscala/reflect/internal/Symbols$Symbol; Types.scala:1105

Make underlyingCache volatile and do double-checked locking in !isCompilerUniverse?

[retronym]

How to build the TSAN enabled JDK with docker. The instrctions would also work for direct usage of Ubuntu.

git clone https://github.com/openjdk/tsan
cd tsan
export D=docker exec builder
docker pull ubuntu:disco
docker run -id --name builder --rm -w /opt -v `pwd`:/host ubuntu:disco bash
$D apt-get update

# https://apt.llvm.org/ "Install" | "Stable Branch"
$D apt-get -y install wget
$D sh -c " wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -"
$D apt-get install -y clang-8 lldb-8 lld-8

# Install Bootstrap JDK
$D apt-get install -y openjdk-12-jdk

# Install tools used in build and libraries referenced during build
$D apt-get install -y git autoconf make zip unzip grep libx11-dev libxext-dev libxrender-dev libxrandr-dev libxtst-dev libxt-dev libcups2-dev libfontconfig1-dev libasound2-dev

# for performance, copy workspace to a directory that docker doesn't share with the host 
$D sh -c "git --git-dir /host/.git archive head | tar -x -C /opt/"

# Configure build
$D bash ./configure --disable-warnings-as-errors --with-jvm-features=tsan --with-toolchain-type=clang --with-toolchain-path=/usr/lib/llvm-8/bin --with-boot-jdk=/usr/lib/jvm/java-12-openjdk-amd64

# Make!
$D make CONF=linux-x86_64-server-release images

# It works
$D build/linux-x86_64-server-release/images/jdk/bin/java -XX:+ThreadSanitizer -version
openjdk version "13-internal" 2019-09-17
OpenJDK Runtime Environment (build 13-internal+0-adhoc..opt)
OpenJDK 64-Bit Server VM (build 13-internal+0-adhoc..opt, interpreted mode)

# Copy binary back to host-shared directory
$D mkdir -p /host/build/linux-x86_64-server-release/images
$D cp -R build/linux-x86_64-server-release/images/jdk /host/build/linux-x86_64-server-release/images/jdk

Use that JDK to run test with TSAN enabled.

$ cat $HOME/tsan-suppressions
called_from_lib:libzip.so
race:java.util.concurrent.locks.AbstractOwnableSynchronizer
race:dequeue
race:FutureTask.report
race:ClassValueMap.loadFromCache
race:java.lang.ClassValue
race:java.lang.ref.SoftReference.get
race:java.lang.ClassValue
race:java.util.concurrent.locks.AbstractQueuedSynchronizer

~/pack/bin/scalac -d /tmp ~/code/scala/test/junit/scala/reflect/runtime/ThreadSafetyTest.scala; 
export JAVA_HOME=<path to buid>/build/linux-x86_64-server-release/images/jdk
export JAVA_OPTS=-XX:+ThreadSanitizer
export TSAN_OPTIONS=second_deadlock_stack=1,suppressions=/home/osboxes/tsan-suppressions
~/pack/bin/scala -nc -cp /tmp scala.reflect.runtime.ThreadSafetyTestMain