This is an automated email from the ASF dual-hosted git repository.
He-Pin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko.git
The following commit(s) were added to refs/heads/main by this push:
new d95b81b0ee fix: keep StashBufferImpl instrumentation hooks from being
inlined (#3616)
d95b81b0ee is described below
commit d95b81b0ee4f4cfe85dd18eefa317ca7c977a741
Author: PJ Fanning <[email protected]>
AuthorDate: Mon Oct 12 07:39:40 2026 +0100
fix: keep StashBufferImpl instrumentation hooks from being inlined (#3616)
* fix: keep StashBufferImpl instrumentation hooks from being inlined
Motivation:
StashBufferImpl's @InternalStableApi hooks (createNode,
interpretUnstashedMessage,
unstashed, stashCleared) are meant for instrumentation agents, but the
Scala 2.13
optimizer inlines them into their callers. The methods stay declared but
are never
invoked, so advice attached to them silently never runs on 2.13.
Modification:
Mark the hooks @noinline, as GraphInterpreter.processPush already is, and
add a
spec that reads the compiled class files and checks every hook still has a
call site.
Result:
Instrumentation on the StashBufferImpl hooks runs on Scala 2.13 as well as
Scala 3.
Tests:
- sbt "+actor-typed-tests/Test/testOnly
org.apache.pekko.actor.typed.internal.StashBufferInstrumentationPointsSpec
org.apache.pekko.actor.typed.scaladsl.StashBufferSpec" - passed on 2.13.18 and
3.3.8
- New spec fails on 2.13 without the fix (createNode,
interpretUnstashedMessage, unstashed, stashCleared not invoked)
- sbt actor-typed/mimaReportBinaryIssues - no issues
References:
None - found while instrumenting stash in micrometer-pekko
* style: apply scalafmt to StashBufferInstrumentationPointsSpec
---
.../StashBufferInstrumentationPointsSpec.scala | 100 +++++++++++++++++++++
.../actor/typed/internal/StashBufferImpl.scala | 5 ++
2 files changed, 105 insertions(+)
diff --git
a/actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/internal/StashBufferInstrumentationPointsSpec.scala
b/actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/internal/StashBufferInstrumentationPointsSpec.scala
new file mode 100644
index 0000000000..8e3933533b
--- /dev/null
+++
b/actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/internal/StashBufferInstrumentationPointsSpec.scala
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.actor.typed.internal
+
+import java.io.DataInputStream
+
+import scala.collection.mutable
+
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+/**
+ * Guards the `@InternalStableApi` hooks in [[StashBufferImpl]] that
instrumentation agents attach bytecode advice to.
+ *
+ * The hooks only work if `StashBufferImpl` still calls them. The Scala 2
optimizer can inline a small private
+ * method into its callers, which leaves the method declared but never
invoked, so advice on it silently never
+ * runs. This spec reads the compiled class file and checks that each hook is
still referenced by a call site.
+ */
+class StashBufferInstrumentationPointsSpec extends AnyWordSpec with Matchers {
+
+ private val className =
"org/apache/pekko/actor/typed/internal/StashBufferImpl"
+
+ private val hooks = List("createNode", "dropHeadForUnstash",
"interpretUnstashedMessage", "unstashed", "stashCleared")
+
+ /** `owner` and its anonymous classes, which is where the unstash iteration
lives. */
+ private def classFiles(owner: String): List[String] =
+ owner ::
+ Iterator
+ .from(1)
+ .map(n => s"$owner$$$$anon$$$n")
+ .takeWhile(name => getClass.getClassLoader.getResource(name + ".class")
ne null)
+ .toList
+
+ /** Names of the methods on `owner` that the class file `classFile` invokes.
*/
+ private def invokedMethodNames(classFile: String, owner: String):
Set[String] = {
+ val in = new
DataInputStream(getClass.getClassLoader.getResourceAsStream(classFile +
".class"))
+ try {
+ in.readInt() // magic
+ in.readUnsignedShort() // minor_version
+ in.readUnsignedShort() // major_version
+ val count = in.readUnsignedShort()
+ val utf8 = mutable.Map.empty[Int, String]
+ val classes = mutable.Map.empty[Int, Int]
+ val nameAndTypes = mutable.Map.empty[Int, Int]
+ val methodRefs = mutable.ListBuffer.empty[(Int, Int)]
+ var i = 1
+ while (i < count) {
+ in.readUnsignedByte() match {
+ case 1 => utf8(i) = in.readUTF()
+ case 7 => classes(i) = in.readUnsignedShort()
+ case 10 | 11 =>
+ methodRefs += ((in.readUnsignedShort(), in.readUnsignedShort()))
+ case 12 =>
+ nameAndTypes(i) = in.readUnsignedShort()
+ in.readUnsignedShort()
+ case 3 | 4 | 9 | 17 | 18 => in.readInt()
+ case 5 | 6 =>
+ in.readLong()
+ i += 1 // long and double entries take two slots
+ case 8 | 16 | 19 | 20 => in.readUnsignedShort()
+ case 15 =>
+ in.readUnsignedByte()
+ in.readUnsignedShort()
+ case tag => fail(s"Unexpected constant pool tag $tag in $classFile")
+ }
+ i += 1
+ }
+ methodRefs.collect {
+ case (classIndex, natIndex) if utf8(classes(classIndex)) == owner =>
utf8(nameAndTypes(natIndex))
+ }.toSet
+ } finally in.close()
+ }
+
+ "StashBufferImpl" should {
+
+ "still call its @InternalStableApi instrumentation hooks" in {
+ val files = classFiles(className)
+ files.size should be > 1
+ val invoked = files.flatMap(invokedMethodNames(_, className)).toSet
+ // a hook accessed from a closure can get an expanded name such as
`...StashBufferImpl$$dropHeadForUnstash`
+ val notInvoked = hooks.filterNot(hook => invoked.exists(name => name ==
hook || name.endsWith("$$" + hook)))
+ notInvoked shouldBe empty
+ }
+ }
+}
diff --git
a/actor-typed/src/main/scala/org/apache/pekko/actor/typed/internal/StashBufferImpl.scala
b/actor-typed/src/main/scala/org/apache/pekko/actor/typed/internal/StashBufferImpl.scala
index b0dc053a57..0b7541e9a5 100644
---
a/actor-typed/src/main/scala/org/apache/pekko/actor/typed/internal/StashBufferImpl.scala
+++
b/actor-typed/src/main/scala/org/apache/pekko/actor/typed/internal/StashBufferImpl.scala
@@ -98,11 +98,13 @@ import pekko.util.OptionVal
}
@InternalStableApi
+ @noinline // Not inlined so that the agent can match the method in the
bytecode
private def createNode(message: T, @nowarn("msg=never used") ctx:
scaladsl.ActorContext[T]): Node[T] = {
new Node(null, message)
}
@InternalStableApi
+ @noinline // Not inlined so that the agent can match the method in the
bytecode
private def dropHeadForUnstash(): Node[T] = {
val message = rawHead
_first = _first.next
@@ -114,6 +116,7 @@ import pekko.util.OptionVal
}
@InternalStableApi
+ @noinline // Not inlined so that the agent can match the method in the
bytecode
private def interpretUnstashedMessage(
behavior: Behavior[T],
ctx: TypedActorContext[T],
@@ -260,10 +263,12 @@ import pekko.util.OptionVal
s"StashBuffer($size/$capacity)"
@InternalStableApi
+ @noinline // Not inlined so that the agent can match the method in the
bytecode
private[pekko] def unstashed(@nowarn("msg=never used") ctx: ActorContext[T],
@nowarn("msg=never used") node: Node[T])
: Unit = ()
@InternalStableApi
+ @noinline // Not inlined so that the agent can match the method in the
bytecode
private def stashCleared(@nowarn("msg=never used") ctx: ActorContext[T]):
Unit = ()
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
