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.

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

Him188/revise on demand scope #2645

Merged
merged 10 commits into from
May 4, 2023
Merged

Him188/revise on demand scope #2645

merged 10 commits into from
May 4, 2023

Conversation

Him188
Copy link
Member

@Him188 Him188 commented Apr 22, 2023

挪到 mirai-core-utils, 重新审查状态设计 (降低复杂度), 增加文档 (注释), 增加测试

close #2633

@Him188 Him188 added the s:core 子系统: mirai-core label Apr 22, 2023
@Him188 Him188 added this to the 2.15.0-RC milestone Apr 22, 2023
@Him188 Him188 force-pushed the him188/revise-on-demand-scope branch from 8c6e6fc to 3bf371a Compare April 22, 2023 12:00
@Him188
Copy link
Member Author

Him188 commented Apr 22, 2023

java.lang.VerifyError: Bad type on operand stack |  
-- | --
  | Exception Details: |  
  | Location: |  
  | net/mamoe/mirai/utils/channels/CoroutineOnDemandReceiveChannel.receiveOrNull(Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @103: getfield |  
  | Reason: |  
  | Type 'java/lang/Object' (current frame, stack[0]) is not assignable to 'net/mamoe/mirai/utils/channels/CoroutineOnDemandReceiveChannel' |  
  | Current Frame: |  
  | bci: @103 |  
  | flags: { } |  
  | locals: { 'net/mamoe/mirai/utils/channels/CoroutineOnDemandReceiveChannel', 'kotlin/coroutines/Continuation', 'java/lang/Object', integer, top, top, top, top, top, top, top, 'java/lang/Object', 'net/mamoe/mirai/utils/channels/CoroutineOnDemandReceiveChannel$receiveOrNull$1', 'java/lang/Object' } |  
  | stack: { 'java/lang/Object' } |  
  | Bytecode:

https://scans.gradle.com/s/esafjm3w3klog/tests/:mirai-core:jvmTest/net.mamoe.mirai.internal.network.component.BotAuthControlTest/auth%20test()?top-execution=1

interesting

@Him188
Copy link
Member Author

Him188 commented Apr 23, 2023

基本好了

  • 删除了 CreatingProducer 状态, 与 ProducerReady 合并, 因此修复了 producer 有可能会启动多次问题 (有可能不是, 具体忘了) (2.15.0-M1 受影响)
  • 修复了 producer 出异常会导致内存泄露问题 (2.15.0-M1 受影响)
  • 修复 expectMore 在 Producing state 不会抛异常的问题
  • 调整了异常的 mesage 以及 cause
  • 调整了 close (以前叫 finish) 的行为, 现在它与 Job.cancel 行为相似, 重复 close 不会抛出异常

producer 抛异常不会立即 close channel, 在下一次 receiveOrNull 时才会 close.
假如 producer 抛异常时碰巧有人调用 close, 异常会被忽略.
如果 producer 抛了异常, 而 receiver 没有 receiveOrNull 而 close, 异常也会被忽略.
异常会被包装成 ProducerFailureException, 像这样:

net.mamoe.mirai.utils.channels.ProducerFailureException: Producer failed to produce a value, see cause
	at net.mamoe.mirai.utils.channels.CoroutineOnDemandReceiveChannel.receiveOrNull(OnDemandChannelImpl.kt:164)
	at net.mamoe.mirai.utils.channels.CoroutineOnDemandReceiveChannel$receiveOrNull$1.invokeSuspend(OnDemandChannelImpl.kt)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.test.TestBuildersKt.runTest$default(Unknown Source)
	at net.mamoe.mirai.utils.channels.OnDemandChannelTest.producer exception(OnDemandChannelTest.kt:273)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: java.util.NoSuchElementException: Oops
	at net.mamoe.mirai.utils.channels.OnDemandChannelTest$producer exception$1$channel$1.invokeSuspend(OnDemandChannelTest.kt:275)
	at net.mamoe.mirai.utils.channels.OnDemandChannelTest$producer exception$1$channel$1.invoke(OnDemandChannelTest.kt)
	at net.mamoe.mirai.utils.channels.OnDemandChannelTest$producer exception$1$channel$1.invoke(OnDemandChannelTest.kt)
	at net.mamoe.mirai.utils.channels.CoroutineOnDemandReceiveChannel$Producer$1.invokeSuspend(OnDemandChannelImpl.kt:46)
	(Coroutine boundary)
	at net.mamoe.mirai.utils.channels.CoroutineOnDemandReceiveChannel.receiveOrNull(OnDemandChannelImpl.kt:162)
	at net.mamoe.mirai.utils.channels.OnDemandChannelTest$producer exception$1.invokeSuspend(OnDemandChannelTest.kt:280)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTestCoroutine$2.invokeSuspend(TestBuilders.kt:212)
Caused by: java.util.NoSuchElementException: Oops
  ...

在 BotAuth 时会最终以 BotAuthorizationException 之类的在 Bot.login 抛出

TODO:

  • [ ] 测试 cancellation
  • [ ] �stress test

UPDATE: 留到第二个 PR 再做

@Him188 Him188 marked this pull request as ready for review May 1, 2023 21:45
@Him188 Him188 force-pushed the him188/revise-on-demand-scope branch 2 times, most recently from b4b7667 to b526df3 Compare May 1, 2023 22:18
@Him188
Copy link
Member Author

Him188 commented May 1, 2023

@Karlatemp 你想 review 吗

@Karlatemp Karlatemp self-requested a review May 2, 2023 10:08
Copy link
Member

@Karlatemp Karlatemp left a comment

Choose a reason for hiding this comment

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

没看具体实现, 看了一遍 test 感觉没啥问题, 实际登录测试过?

@Him188
Copy link
Member Author

Him188 commented May 4, 2023

e: file:///Users/runner/work/mirai/mirai/mirai-core-
utils/src/commonMain/kotlin/UtilsLogger.kt:69:42 This can only be used in tests.

e: file:///Users/runner/work/mirai/mirai/mirai-core-utils/src/commonMain/kotlin/UtilsLogger.kt:69:42 This can only be used in tests

@Him188 Him188 requested a review from Karlatemp May 4, 2023 09:07
Him188 added 10 commits May 4, 2023 13:11
also added delegations in mirai-core and mirai-core-api.
and rename OnDemandValueScope to OnDemandSendChannel and OnDemandReceiveChannel
[core] Start producer coroutine immediately on `expectMore` and yield

Improve docs for OnDemandChannel

Rename factory function `OnDemandReceiveChannel` to  `OnDemandChannel` to better cover its meaning

Create deferred lazily in Producing state

Rename ProducerState to ChannelState

fix atomicfu bug receiveOrNull

Add docs (WIP, to be rebased)

[core] OnDemandChannel: Catch Throwable in `receiveOrNull` to prevent possible failures
[core] OnDemandChannel: Ensure channel close and emit shares same behavior

[core] OnDemandChannel: Ensure channel is closed properly when producer throws an exception

[core] OnDemandChannel: Ensure coroutine scope cancelled when producer coroutine throws exception,

also added more tests
@Him188 Him188 force-pushed the him188/revise-on-demand-scope branch from 60ce585 to 909985d Compare May 4, 2023 12:12
@Him188 Him188 merged commit bda5d54 into dev May 4, 2023
@Him188 Him188 deleted the him188/revise-on-demand-scope branch May 4, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:core 子系统: mirai-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants