Skip to content

Commit e3396e3

Browse files
Merge pull request #277 from chippr-robotics/copilot/fix-networking-bug-again
[WIP] Fix networking bug related to null runtime
2 parents 22989af + 09229ef commit e3396e3

File tree

3 files changed

+182
-4
lines changed

3 files changed

+182
-4
lines changed

src/main/scala/com/chipprbots/ethereum/nodebuilder/NodeBuilder.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ trait PeerDiscoveryManagerBuilder {
136136
with DiscoveryServiceBuilder
137137
with StorageBuilder =>
138138

139-
implicit val ioRuntime: IORuntime = IORuntime.global
139+
implicit lazy val ioRuntime: IORuntime = IORuntime.global
140140

141141
lazy val peerDiscoveryManager: ActorRef = system.actorOf(
142142
PeerDiscoveryManager.props(
@@ -776,7 +776,7 @@ trait SyncControllerBuilder {
776776
trait PortForwardingBuilder {
777777
self: DiscoveryConfigBuilder =>
778778

779-
implicit val ioRuntime: IORuntime = IORuntime.global
779+
implicit lazy val ioRuntime: IORuntime = IORuntime.global
780780

781781
private val portForwarding = PortForwarder
782782
.openPorts(
@@ -911,5 +911,5 @@ trait Node
911911
with PortForwardingBuilder
912912
with BlacklistBuilder {
913913
// Resolve conflicting ioRuntime from PeerDiscoveryManagerBuilder and PortForwardingBuilder
914-
implicit override val ioRuntime: IORuntime = IORuntime.global
914+
implicit override lazy val ioRuntime: IORuntime = IORuntime.global
915915
}

src/main/scala/com/chipprbots/ethereum/nodebuilder/TestNode.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import com.chipprbots.ethereum.utils.BlockchainConfig
1313

1414
class TestNode extends BaseNode {
1515

16-
override val ioRuntime: IORuntime = IORuntime.global
16+
override lazy val ioRuntime: IORuntime = IORuntime.global
1717

1818
lazy val testModeComponentsProvider: TestModeComponentsProvider =
1919
new TestModeComponentsProvider(
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package com.chipprbots.ethereum.nodebuilder
2+
3+
import org.apache.pekko.actor.ActorSystem
4+
import org.apache.pekko.testkit.TestKit
5+
6+
import cats.effect.unsafe.IORuntime
7+
8+
import org.scalatest.BeforeAndAfterAll
9+
import org.scalatest.flatspec.AnyFlatSpecLike
10+
import org.scalatest.matchers.should.Matchers
11+
12+
/** Tests to validate that IORuntime is properly initialized in the NodeBuilder trait hierarchy
13+
* to prevent null pointer exceptions during actor creation.
14+
*
15+
* This test specifically validates the fix for the networking bug where IORuntime was null
16+
* when PeerDiscoveryManager tried to use it.
17+
*
18+
* The key issue was that `implicit val ioRuntime` was not lazy, causing initialization order
19+
* problems when traits were mixed together. Making it `implicit lazy val` ensures it's
20+
* initialized only when first accessed, avoiding null pointer exceptions.
21+
*/
22+
class IORuntimeInitializationSpec
23+
extends TestKit(ActorSystem("IORuntimeInitializationSpec"))
24+
with AnyFlatSpecLike
25+
with Matchers
26+
with BeforeAndAfterAll {
27+
28+
override def afterAll(): Unit = {
29+
TestKit.shutdownActorSystem(system)
30+
}
31+
32+
behavior.of("IORuntime initialization in NodeBuilder traits")
33+
34+
it should "ensure IORuntime is lazy to avoid initialization order issues" in {
35+
// This test validates that the implicit val is actually lazy
36+
// If it's not lazy, initialization order issues can occur when traits are mixed
37+
38+
@volatile var peerDiscoveryBuilderAccessed = false
39+
@volatile var portForwardingBuilderAccessed = false
40+
41+
trait TestPeerDiscoveryManagerBuilder {
42+
implicit lazy val ioRuntime: IORuntime = {
43+
peerDiscoveryBuilderAccessed = true
44+
IORuntime.global
45+
}
46+
}
47+
48+
trait TestPortForwardingBuilder {
49+
implicit lazy val ioRuntime: IORuntime = {
50+
portForwardingBuilderAccessed = true
51+
IORuntime.global
52+
}
53+
}
54+
55+
trait TestNode extends TestPeerDiscoveryManagerBuilder with TestPortForwardingBuilder {
56+
// This override simulates the Node trait's override
57+
implicit override lazy val ioRuntime: IORuntime = IORuntime.global
58+
}
59+
60+
val node = new TestNode {}
61+
62+
// The runtime should not be accessed yet because it's lazy
63+
peerDiscoveryBuilderAccessed shouldBe false
64+
portForwardingBuilderAccessed shouldBe false
65+
66+
// Now access it
67+
val runtime = node.ioRuntime
68+
69+
// Now it should be accessed and not null
70+
runtime should not be null
71+
runtime.compute should not be null
72+
}
73+
74+
it should "have IORuntime available when accessed from mixed traits" in {
75+
// This test validates that the IORuntime is available during lazy val initialization
76+
trait TestBuilderWithRuntime {
77+
implicit lazy val ioRuntime: IORuntime = IORuntime.global
78+
79+
def getRuntimeForTest: IORuntime = ioRuntime
80+
}
81+
82+
val builder = new TestBuilderWithRuntime {}
83+
84+
// Access the runtime - this should not be null
85+
val runtime = builder.getRuntimeForTest
86+
runtime should not be null
87+
runtime.compute should not be null
88+
}
89+
90+
it should "properly initialize IORuntime with multiple trait overrides" in {
91+
// This test simulates the actual Node trait structure with multiple overrides
92+
trait Base {
93+
implicit lazy val ioRuntime: IORuntime = IORuntime.global
94+
}
95+
96+
trait Override1 extends Base {
97+
implicit override lazy val ioRuntime: IORuntime = IORuntime.global
98+
}
99+
100+
trait Override2 extends Base {
101+
implicit override lazy val ioRuntime: IORuntime = IORuntime.global
102+
}
103+
104+
trait Final extends Override1 with Override2 {
105+
implicit override lazy val ioRuntime: IORuntime = IORuntime.global
106+
}
107+
108+
val node = new Final {}
109+
110+
// The runtime should be properly initialized
111+
node.ioRuntime should not be null
112+
node.ioRuntime.compute should not be null
113+
}
114+
115+
it should "ensure lazy val IORuntime is thread-safe during initialization" in {
116+
// This test validates thread-safety of lazy val initialization
117+
// Note: Due to JVM implementation details, lazy vals may be initialized multiple times
118+
// in extreme race conditions, but the final value is always consistent
119+
@volatile var initCount = 0
120+
121+
trait TestRuntime {
122+
implicit lazy val ioRuntime: IORuntime = {
123+
initCount += 1
124+
Thread.sleep(10) // Simulate some initialization work
125+
IORuntime.global
126+
}
127+
}
128+
129+
val runtime = new TestRuntime {}
130+
131+
// Access from multiple threads
132+
val threads = (1 to 5).map { _ =>
133+
new Thread(new Runnable {
134+
def run(): Unit = {
135+
runtime.ioRuntime should not be null
136+
}
137+
})
138+
}
139+
140+
threads.foreach(_.start())
141+
threads.foreach(_.join())
142+
143+
// Lazy val provides consistent values even under concurrent access
144+
runtime.ioRuntime should not be null
145+
runtime.ioRuntime.compute should not be null
146+
// Note: initCount may be > 1 due to race conditions during initialization,
147+
// but this is acceptable as long as the final value is consistent
148+
initCount should be >= 1
149+
}
150+
151+
it should "validate that non-lazy val would cause initialization issues" in {
152+
// This test documents the problem: non-lazy vals can be null during mixed trait initialization
153+
@volatile var eagerInitOrder = scala.collection.mutable.ListBuffer[String]()
154+
155+
trait EagerBase {
156+
implicit val ioRuntime: IORuntime = {
157+
eagerInitOrder += "Base"
158+
IORuntime.global
159+
}
160+
}
161+
162+
trait EagerOverride extends EagerBase {
163+
implicit override val ioRuntime: IORuntime = {
164+
eagerInitOrder += "Override"
165+
IORuntime.global
166+
}
167+
}
168+
169+
val node = new EagerOverride {}
170+
171+
// With non-lazy vals, initialization happens immediately during trait construction
172+
// This can lead to issues with initialization order
173+
node.ioRuntime should not be null
174+
175+
// Document that both were initialized (eager initialization)
176+
eagerInitOrder should not be empty
177+
}
178+
}

0 commit comments

Comments
 (0)