Cyclic object initialization between Predef and collection.immutable.Vector

[EnzeXing]

Description

The source code of the standard library includes object initialization cycles, including a cycle between Predef and collection.immutable.Vector, which could lead to deadlock when two threads initialize them simultaneously.

Test

deadlock.scala

``` object Test extends App { val createPredef = new Runnable { def run = { val _ = Predef; } } val createVector = new Runnable { def run = { val _ = scala.collection.immutable.Vector; } } val t1 = new Thread(createPredef) val t2 = new Thread(createVector) t1.start() t2.start() t1.join() t2.join() } ```

Reproduction steps

Scala version: (2.13.15)

git clone https://github.com/scala/scala.git
cd scala
git checkout 2.13.x
sbt publishLocal
scala-cli -S 2.13.15-bin-SNAPSHOT deadlock.scala --main-class Test

Problem

The test deadlocks due to cyclic initialization between Predef and collection.immutable.Vector. Specifically, Predef triggers initialization of Vector through

   |├── object Predef extends LowPriorityImplicits {    [ Predef.scala:106 ]
   |│   ^
   |├── scala.`package`                         // to force scala package object to be seen.    [ Predef.scala:151 ]
   |│   ^^^^^^^^^^^^^^^
   |├── package object scala {  [ package.scala:19 ]
   |│   ^
   |└── val Vector = scala.collection.immutable.Vector  [ package.scala:101 ]
   |        

and the constructor of Vector contains another call to Predef.augmentString, as shown in Vector.tasty

private[this] val defaultApplyPreferredMaxLength: scala.Int = try scala.Predef.augmentString(java.lang.System.getProperty("scala.collection.immutable.Vector.defaultApplyPreferredMaxLength", "250")).toInt catch {
      case _: java.lang.SecurityException =>
        250
    }

The warning is given by the global initialization checker that is under development to fix initialization warning in standard tasty library (https://github.com/scala/scala3/issues/18882), and this issue is linked to a PR with a fix to manually inline the call to Predef.augmentString (https://github.com/scala/scala/pull/10793). Our current conclusion is that such calls must be inlined to ensure soundness, which may happen in released build.

To reproduce the warning using the global initialization checker, apply the following patch to Dotty main branch:

diff --git a/compiler/src/dotty/tools/dotc/transform/init/Objects.scala b/compiler/src/dotty/tools/dotc/transform/init/Objects.scala
index bfa684eef8..3bb7b23095 100644
--- a/compiler/src/dotty/tools/dotc/transform/init/Objects.scala
+++ b/compiler/src/dotty/tools/dotc/transform/init/Objects.scala
@@ -864,8 +864,10 @@ class Objects(using Context @constructorOnly):
       Bottom

     case Bottom =>
-      if field.isStaticObject then ObjectRef(field.moduleClass.asClass)
-      else Bottom
+      if field.isStaticObject then
+        accessObject(field.moduleClass.asClass)
+      else
+        Bottom

     case ValueSet(values) =>
       values.map(ref => select(ref, field, receiver)).join

and in sbt:

sbt> set ThisBuild/Build.scala2Library := Build.Scala2LibraryTasty
sbt> scala3-compiler-bootstrapped/scalac -d out -Ysafe-init-global tests/init-global/pos/deadlock2.scala

The checker will report the following warning:

-- Warning: out/bootstrap/scala2-library-bootstrapped/scala-3.5.1-RC1-bin-SNAPSHOT-nonbootstrapped/src_managed/main/scala-library-src/scala/collection/immutable/Vector.scala:34:7
34 |object Vector extends StrictOptimizedSeqFactory[Vector] {
   |       ^
   |Cyclic initialization: object Vector -> object Predef -> package object scala -> object Vector. Calling trace:
   |├── object Predef extends LowPriorityImplicits {    [ Predef.scala:106 ]
   |│   ^
   |├── scala.`package`                         // to force scala package object to be seen.    [ Predef.scala:151 ]
   |│   ^^^^^^^^^^^^^^^
   |├── object Vector extends StrictOptimizedSeqFactory[Vector] {   [ Vector.scala:34 ]
   |│   ^
   |├── try System.getProperty("scala.collection.immutable.Vector.defaultApplyPreferredMaxLength",  [ Vector.scala:84 ]
   |│       ^
   |├── package object scala {  [ package.scala:19 ]
   |│   ^
   |└── val Vector = scala.collection.immutable.Vector  [ package.scala:101 ]
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1 warning found

[olhotak]

Note that the deadlock happens only when using the publishLocal version, not the released version.

The following does not deadlock:

scala-cli -S 2.13.14 deadlock.scala --main-class Test

The following does deadlock:

git checkout v2.13.14
sbt publishLocal
scala-cli -S 2.13.14-bin-SNAPSHOT deadlock.scala --main-class Test

Both are 2.13.14, but one is the released library and the other is publishLocal.

We think this is because the build process for the released version does more inlining than the publishLocal version.

In particular, the call to augmentString appears in the bytecode of the publishLocal version but does not appear in the bytecode of the released version; it has been inlined.

[som-snytt]

I fixed the PR comment to reference this ticket. Comments there note that it is a difference with inlining (qua optimization).

Edit: setupPublishCore enables optimization before publishLocal, for local testing.

Edit again: maybe the previous comment was using "royal we" to express the linked PR comments.