Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Trustchain failing from command line #57

Closed
wants to merge 6 commits into from
Closed

Fix Trustchain failing from command line #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
752d8a2
Pass an optional CoroutineDispatcher to an Overlay's load() function …
Nov 18, 2021
c382a35
Merge branch 'master' of https://github.com/Tribler/kotlin-ipv8 into …
Jan 18, 2022
3c0ebd5
Re-add load function without parameters for compatibility with older …
Jan 18, 2022
d9e7d63
Fix typos in Overlay.kt
InvictusRMC Jan 18, 2022
fa7de7e
Add an explicit testdispatcher to broken tests.
Jan 18, 2022
4f564d4
Merge branch 'master' of https://github.com/Tribler/kotlin-ipv8 into …
Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions demo-jvm/src/main/java/nl/tudelft/ipv8/jvm/demo/Application.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import java.util.*
import kotlin.math.roundToInt

class Application {
private val scope = CoroutineScope(Dispatchers.Default)
private val dispatcher = Dispatchers.Default
private val scope = CoroutineScope(dispatcher)
private val logger = KotlinLogging.logger {}

fun run() {
Expand Down Expand Up @@ -78,7 +79,7 @@ class Application {
), walkerInterval = 1.0)

val ipv8 = IPv8(endpoint, config, myPeer)
ipv8.start()
ipv8.start(dispatcher)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1/4) Now we're passing Dispatchers.Default.


scope.launch {
while (true) {
Expand Down
6 changes: 3 additions & 3 deletions ipv8/src/main/java/nl/tudelft/ipv8/Community.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ abstract class Community : Overlay {
messageHandlers[MessageId.EVA_ERROR] = ::onEVAErrorPacket
}

override fun load() {
super.load()
override fun load(dispatcher: CoroutineDispatcher) {
super.load(dispatcher)

logger.info { "Loading " + javaClass.simpleName + " for peer " + myPeer.mid }

Expand All @@ -69,7 +69,7 @@ abstract class Community : Overlay {
network.blacklist.addAll(DEFAULT_ADDRESSES)

job = SupervisorJob()
scope = CoroutineScope(Dispatchers.Main + job)
scope = CoroutineScope(dispatcher + job)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(4/4) This will now be Dispatchers.Default as opposed to being Dispatchers.Main in the demo application.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is precisely what we want to be doing. The reason this issue exists is because a command line application does not (necessarily) have a Main dispatcher. The Main dispatcher is "confined to the Main thread operating with UI objects.". Please refer to issue #57, where I've described the issue more explicitly. By making the dispatcher an optional argument, existing applications should not break. But future console applications should refrain from using a Main dispatcher.


if (evaProtocolEnabled)
evaProtocol = EVAProtocol(this, scope)
Expand Down
4 changes: 2 additions & 2 deletions ipv8/src/main/java/nl/tudelft/ipv8/IPv8.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class IPv8(
return overlays[T::class.java] as? T
}

fun start() {
fun start(dispatcher: CoroutineDispatcher = Dispatchers.Main) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(2/4) Resulting in this not being Dispatchers.Main

if (isStarted()) throw IllegalStateException("IPv8 has already started")

isStarted = true
Expand All @@ -51,7 +51,7 @@ class IPv8(
overlay.endpoint = endpoint
overlay.network = network
overlay.maxPeers = overlayConfiguration.maxPeers
overlay.load()
overlay.load(dispatcher)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(3/4) We're now passing Dispatchers.Default.


overlays[overlayClass] = overlay

Expand Down
11 changes: 10 additions & 1 deletion ipv8/src/main/java/nl/tudelft/ipv8/Overlay.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package nl.tudelft.ipv8

import kotlinx.coroutines.CoroutineDispatcher
import nl.tudelft.ipv8.messaging.EndpointAggregator
import nl.tudelft.ipv8.messaging.EndpointListener
import nl.tudelft.ipv8.peerdiscovery.Network
Expand All @@ -22,12 +23,20 @@ interface Overlay : EndpointListener {
get() = myPeer.lamportTimestamp

/**
* Called to inintialize this overlay.
* Called to initialize this overlay.
*/
fun load() {
endpoint.addListener(this)
}


/**
* Called to initialize this overlay.
*/
fun load(dispatcher: CoroutineDispatcher) {
endpoint.addListener(this)
}

/**
* Called when this overlay needs to be shut down.
*/
Expand Down
4 changes: 2 additions & 2 deletions ipv8/src/main/java/nl/tudelft/ipv8/attestation/trustchain/TrustChainCommunity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ open class TrustChainCommunity(
messageHandlers[MessageId.EMPTY_CRAWL_RESPONSE] = ::onEmptyCrawlResponsePacket
}

override fun load() {
super.load()
override fun load(dispatcher: CoroutineDispatcher) {
super.load(dispatcher)
crawler.trustChainCommunity = this
}

Expand Down
2 changes: 1 addition & 1 deletion ipv8/src/test/java/nl/tudelft/ipv8/BaseCommunityTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ private val lazySodium = LazySodiumJava(SodiumJava())

@OptIn(ExperimentalCoroutinesApi::class)
abstract class BaseCommunityTest {
private val testDispatcher = TestCoroutineDispatcher()
protected val testDispatcher = TestCoroutineDispatcher()

@Before
fun setUp() {
Expand Down
4 changes: 3 additions & 1 deletion ipv8/src/test/java/nl/tudelft/ipv8/CommunityTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import io.mockk.every
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
import kotlinx.coroutines.ExperimentalCoroutinesApi
import nl.tudelft.ipv8.keyvault.*
import nl.tudelft.ipv8.messaging.Address
import nl.tudelft.ipv8.messaging.Packet
Expand Down Expand Up @@ -119,6 +120,7 @@ class CommunityTest : BaseCommunityTest() {
verify { community.onIntroductionResponse(any(), any()) }
}

@OptIn(ExperimentalCoroutinesApi::class)
@Test
fun loadAndUnload() {
val myPrivateKey = getPrivateKey()
Expand All @@ -127,7 +129,7 @@ class CommunityTest : BaseCommunityTest() {

val community = getCommunity()

community.load()
community.load(testDispatcher)
Assert.assertEquals(1, community.network.blacklistMids.size)
Assert.assertEquals(myPeer.mid, community.network.blacklistMids.first())
community.unload()
Expand Down
4 changes: 3 additions & 1 deletion ipv8/src/test/java/nl/tudelft/ipv8/attestation/trustchain/TrustChainCommunityTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.mockk.Called
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.ExperimentalCoroutinesApi
import nl.tudelft.ipv8.IPv4Address
import nl.tudelft.ipv8.BaseCommunityTest
import nl.tudelft.ipv8.attestation.trustchain.payload.HalfBlockPayload
Expand All @@ -21,6 +22,7 @@ import java.math.BigInteger
import java.util.*

class TrustChainCommunityTest : BaseCommunityTest() {
@OptIn(ExperimentalCoroutinesApi::class)
private fun getCommunity(): TrustChainCommunity {
val settings = TrustChainSettings()
val store = mockk<TrustChainStore>(relaxed = true)
Expand All @@ -29,7 +31,7 @@ class TrustChainCommunityTest : BaseCommunityTest() {
community.endpoint = getEndpoint()
community.network = Network()
community.maxPeers = 20
community.load()
community.load(testDispatcher)
return community
}

Expand Down
16 changes: 8 additions & 8 deletions ipv8/src/test/java/nl/tudelft/ipv8/messaging/eva/CommunityEVATest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun sendEVAWriteRequest() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

val previousRequest = community.myPeer.lastRequest

Expand All @@ -241,7 +241,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun sendEVAAcknowledgement() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

val previousRequest = community.myPeer.lastRequest

Expand All @@ -264,7 +264,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun sendEVAData() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

val previousRequest = community.myPeer.lastRequest

Expand All @@ -288,7 +288,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun sendEVAError() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

val previousRequest = community.myPeer.lastRequest

Expand All @@ -310,7 +310,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun setOnEVASendCompleteCallback() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

assertEquals(null, community.evaProtocol?.onSendCompleteCallback)
community.setOnEVASendCompleteCallback { _, _, _ -> }
Expand All @@ -322,7 +322,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun setOnEVAReceiveCompleteCallback() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

assertEquals(null, community.evaProtocol?.onReceiveCompleteCallback)
community.setOnEVAReceiveCompleteCallback { _, _, _, _ -> }
Expand All @@ -334,7 +334,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun setOnEVAReceiveProgressCallback() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

assertEquals(null, community.evaProtocol?.onReceiveProgressCallback)
community.setOnEVAReceiveProgressCallback { _, _, _ -> }
Expand All @@ -346,7 +346,7 @@ class CommunityEVATest : BaseCommunityTest() {
@Test
fun setOnEVAErrorCallback() {
val community = getCommunity()
community.load()
community.load(testDispatcher)

assertEquals(null, community.evaProtocol?.onErrorCallback)
community.setOnEVAErrorCallback { _, _ -> }
Expand Down
Loading