Improve core network stability:

Close network and do logout only if networkInitialized

core, network: Guard changing state

core, network: Protect state overriding when setting a state

Always close NetworkHandlerSupport even if CommonNetworkHandler is in CLOSED state

mirai-core/src/commonMain/kotlin/AbstractBot.kt

@@ -57,7 +57,10 @@ internal abstract class AbstractBot constructor(
                     logger.info { "Bot cancelled" + throwable?.message?.let { ": $it" }.orEmpty() }
 
                     kotlin.runCatching {
-                        network.close(throwable)
+                        val bot = bot
+                        if (bot is AbstractBot && bot.networkInitialized) {
+                            bot.network.close(throwable)
+                        }
                     }.onFailure {
                         if (it !is CancellationException) logger.error(it)
                     }
@@ -121,7 +124,7 @@ internal abstract class AbstractBot constructor(
     ///////////////////////////////////////////////////////////////////////////
 
     @Volatile
-    private var networkInitialized = false
+    var networkInitialized = false
     val network: NetworkHandler by lazy {
         networkInitialized = true
         createNetworkHandler()

mirai-core/src/commonMain/kotlin/QQAndroidBot.kt

@@ -6,7 +6,6 @@
  *
  * https://github.com/mamoe/mirai/blob/dev/LICENSE
  */
-@file:Suppress("EXPERIMENTAL_API_USAGE", "INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
 
 package net.mamoe.mirai.internal
 
@@ -73,14 +72,18 @@ internal open class QQAndroidBot constructor(
 
     override fun close(cause: Throwable?) {
         if (!this.isActive) return
-        runBlocking {
-            try { // this may not be very good but
-                withTimeoutOrNull(5.seconds) {
-                    components[SsoProcessor].logout(network)
+
+        if (networkInitialized) {
+            runBlocking {
+                try { // this may not be very good but
+                    withTimeoutOrNull(5.seconds) {
+                        components[SsoProcessor].logout(network)
+                    }
+                } catch (ignored: Exception) {
                 }
-            } catch (ignored: Exception) {
             }
         }
+
         super.close(cause)
     }
 

mirai-core/src/commonMain/kotlin/network/handler/CommonNetworkHandler.kt

@@ -162,9 +162,9 @@ internal abstract class CommonNetworkHandler<Conn>(
     ///////////////////////////////////////////////////////////////////////////
 
     override fun close(cause: Throwable?) {
+        super.close(cause) // cancel coroutine scope
         if (state == NetworkHandler.State.CLOSED) return // quick check if already closed
         if (setState { StateClosed(cause) } == null) return // atomic check
-        super.close(cause) // cancel coroutine scope
     }
 
     init {

mirai-core/src/commonMain/kotlin/network/handler/NetworkHandlerSupport.kt

@@ -31,6 +31,7 @@ import net.mamoe.mirai.utils.*
 import net.mamoe.mirai.utils.Either.Companion.fold
 import kotlin.coroutines.CoroutineContext
 import kotlin.coroutines.EmptyCoroutineContext
+import kotlin.jvm.Volatile
 import kotlin.reflect.KClass
 
 /**
@@ -279,6 +280,9 @@ internal abstract class NetworkHandlerSupport(
     private val lock = reentrantLock()
     internal val lockForSetStateWithOldInstance = SynchronizedObject()
 
+    @Volatile
+    private var changingState: KClass<out BaseStateImpl>? = null
+
     /**
      * This can only be called by [setState] or in tests. Note:
      */
@@ -290,30 +294,56 @@ internal abstract class NetworkHandlerSupport(
             if (old::class == newType) return@withLock null // already set to expected state by another thread. Avoid replications.
             if (old.correspondingState == NetworkHandler.State.CLOSED) return@withLock null // CLOSED is final.
 
-            val stateObserver = context.getOrNull(StateObserver)
-
-            val impl = try {
-                new()
-            } catch (e: Throwable) {
-                stateObserver?.exceptionOnCreatingNewState(this, old, e)
-                throw e
+            val changingState = changingState
+            if (changingState != null) {
+                if (changingState == newType) {
+                    // no duplicates
+                    return null
+                } else {
+                    error("New state ${newType.simpleName} clashes with current switching process, changingState = ${changingState.simpleName}.")
+                }
             }
 
-            check(old !== impl) { "Old and new states cannot be the same." }
+            this.changingState = newType
+
+            try {
+
+                val stateObserver = context.getOrNull(StateObserver)
 
-            stateObserver?.beforeStateChanged(this, old, impl)
+                val impl = try {
+                    new()
+                } catch (e: Throwable) {
+                    stateObserver?.exceptionOnCreatingNewState(this, old, e)
+                    throw e
+                }
 
-            // We should startState before expose it publicly because State.resumeConnection may wait for some jobs that are launched in startState.
-            // We cannot close old state before changing the 'public' _state to be the new one, otherwise every client will get some kind of exceptions (unspecified, maybe CancellationException).
-            impl.startState() // launch jobs
-            _state = impl // update current state
-            old.cancel(StateSwitchingException(old, impl)) // close old
-            impl.afterUpdated() // now do post-update things.
+                try {
+                    check(old !== impl) { "Old and new states cannot be the same." }
 
-            stateObserver?.stateChanged(this, old, impl) // notify observer
-            _stateChannel.trySend(impl.correspondingState) // notify selector
+                    stateObserver?.beforeStateChanged(this, old, impl)
 
-            return@withLock impl
+                    // We should startState before expose it publicly because State.resumeConnection may wait for some jobs that are launched in startState.
+                    // We cannot close old state before changing the 'public' _state to be the new one, otherwise every client will get some kind of exceptions (unspecified, maybe CancellationException).
+                    impl.startState() // launch jobs
+                } catch (e: Throwable) {
+                    throw e
+                }
+
+                // No further change
+
+                _state = impl // update current state
+                // After _state is updated, we are safe.
+
+                old.cancel(StateSwitchingException(old, impl)) // close old
+                impl.afterUpdated() // now do post-update things.
+
+                stateObserver?.stateChanged(this, old, impl) // notify observer
+                _stateChannel.trySend(impl.correspondingState) // notify selector
+
+                return@withLock impl
+            } finally {
+                this.changingState = null
+            }
         }
 
     final override suspend fun resumeConnection() {

mirai-core/src/commonMain/kotlin/network/handler/state/LoggingStateObserver.kt

@@ -51,7 +51,7 @@ internal class LoggingStateObserver(
         previousState: NetworkHandlerSupport.BaseStateImpl,
         exception: Throwable,
     ) {
-        logger.debug { "State changed: ${previousState.correspondingState} -> $exception" }
+        logger.debug { "State exception: ${previousState.correspondingState} -> $exception" }
     }
 
     override suspend fun beforeStateResume(networkHandler: NetworkHandler, state: NetworkHandlerSupport.BaseStateImpl) {

mirai-core/src/commonTest/kotlin/network/framework/AbstractCommonNHTest.kt

@@ -100,8 +100,6 @@ internal expect abstract class AbstractCommonNHTest() :
 
     val conn: PlatformConn
 
-    override val network: TestCommonNetworkHandler
-
     override val factory: NetworkHandlerFactory<TestCommonNetworkHandler>
 
     protected fun removeOutgoingPacketEncoder()

mirai-core/src/jvmBaseTest/kotlin/network/framework/AbstractCommonNHTest.kt

@@ -20,9 +20,6 @@ import kotlin.test.AfterTest
  */
 internal actual abstract class AbstractCommonNHTest actual constructor() :
     AbstractRealNetworkHandlerTest<TestCommonNetworkHandler>() {
-    actual override val network: TestCommonNetworkHandler by lazy {
-        factory.create(createContext(), createAddress())
-    }
 
     private val startedDispatchers = mutableListOf<ExecutorCoroutineDispatcher>()
 

mirai-core/src/nativeTest/kotlin/network/framework/AbstractCommonNHTest.kt

@@ -19,10 +19,6 @@ import net.mamoe.mirai.internal.network.handler.NetworkHandlerFactory
 internal actual abstract class AbstractCommonNHTest actual constructor() :
     AbstractRealNetworkHandlerTest<TestCommonNetworkHandler>() {
 
-    actual override val network: TestCommonNetworkHandler by lazy {
-        factory.create(createContext(), createAddress())
-    }
-
     actual override val factory: NetworkHandlerFactory<TestCommonNetworkHandler> =
         NetworkHandlerFactory<TestCommonNetworkHandler> { context, address ->
             object : TestCommonNetworkHandler(bot, context, address) {