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

Reduce reachable references of disposed invokeOnTimeout handle #3353

Merged
merged 2 commits into from
Jun 30, 2022

Conversation

qwwdfsad
Copy link
Contributor

Fixes #3351

@qwwdfsad qwwdfsad requested a review from dkhalanskyjb June 29, 2022 15:32
@qwwdfsad
Copy link
Contributor Author

qwwdfsad commented Jun 29, 2022

I cannot write a proper unit test though, but we've discussed it with K/N folks and it seems like there is a potential API for that fun Any.getObjectReferenceFields() = List<Any>. I'll keep you updated about that.

UPD: https://youtrack.jetbrains.com/issue/KT-53022

Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb left a comment

Choose a reason for hiding this comment

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

Makes sense. The suggestions are just for consideration, disregard them if you disagree.

Comment on lines +53 to +55
val disposableBlock = DisposableBlock(block)
worker.executeAfter(timeMillis.toMicrosSafe(), disposableBlock)
return disposableBlock
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val disposableBlock = DisposableBlock(block)
worker.executeAfter(timeMillis.toMicrosSafe(), disposableBlock)
return disposableBlock
DisposableBlock(block).also { worker.executeAfter(timeMillis.toMicrosSafe(), it) }

The variable name does not seem to be informative.

@qwwdfsad qwwdfsad merged commit 7934032 into develop Jun 30, 2022
@qwwdfsad qwwdfsad deleted the fix-native-withTimeout-leak branch June 30, 2022 13:29
pablobaxter pushed a commit to pablobaxter/kotlinx.coroutines that referenced this pull request Sep 14, 2022
…n#3353)

* Reduce reachable references of disposed invokeOnTimeout handle

Fixes Kotlin#3351
pablobaxter added a commit to pablobaxter/kotlinx.coroutines that referenced this pull request Sep 14, 2022
…to baxter/upstream-flow-timeout

* origin/baxter/upstream-flow-timeout: (328 commits)
  Commit API dump
  Cleanup API, update knit
  Fix typo in runTest method docs (Kotlin#3417)
  Update coroutines-and-channels.md (Kotlin#3410)
  chore: update the website's release step (Kotlin#3397)
  ktl-695 chore: support Dokka HTML customization (Kotlin#3388)
  update: KT-50122 adding kotlinx.dependencies
  Improve bump-version.sh (Kotlin#3365)
  Fix documentation for `DEBUG_PROPERTY_VALUE_OFF` (Kotlin#3389)
  feat: moving coroutines hands-on to docs (Kotlin#3369)
  Version 1.6.4
  Improve CoroutineDispatcher documentation (Kotlin#3359)
  Update binary compatibility validator to 0.11.0 (Kotlin#3362)
  Add a scope for launching background work in tests (Kotlin#3348)
  Fix debug module publication with shadow plugin (Kotlin#3357)
  Comply with Subscriber rule 2.7 in the `await*` impl (Kotlin#3360)
  Update readme (Kotlin#3343)
  Reduce reachable references of disposed invokeOnTimeout handle (Kotlin#3353)
  breakleg; knit validation fix
  Additional comment in CoroutineScheduler
  ...

# Conflicts:
#	README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants