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

Jump to bottom

Unsynchronized access to ThreadSafeHeap in K/N #3547

Closed
qwwdfsad opened this issue Dec 5, 2022 · 2 comments
Closed

Unsynchronized access to ThreadSafeHeap in K/N #3547

qwwdfsad opened this issue Dec 5, 2022 · 2 comments
Labels
bug native

Comments

@qwwdfsad
Copy link
Contributor

qwwdfsad commented Dec 5, 2022

https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/common/src/EventLoop.common.kt#L441

Note that @Synchronized has no effect on K/N
qwwdfsad added a commit that referenced this issue Dec 7, 2022
@qwwdfsad
Make EventLoopImplBase properly synchronized on Kotlin/Native
ab260d9 
Fixes #3547
@qwwdfsad qwwdfsad mentioned this issue Dec 7, 2022
Merged
qwwdfsad added a commit that referenced this issue Dec 27, 2022
@qwwdfsad
Make EventLoopImplBase properly synchronized on Kotlin/Native (#3550)
c194877 
Fixes #3547
@05nelsonm
Copy link

I know this was marked as fixed in #3550 , but wouldn't you also need to change anything marked as @Volatile to an AtomicRef?

kotlinx.coroutines/kotlinx-coroutines-core/common/src/EventLoop.common.kt

Line 418 in 725c46c

@Volatile

@qwwdfsad
Copy link
Contributor Author

qwwdfsad commented Jan 2, 2023 •
edited
Loading

Nice catch!
This is just to suppress a TSAN warning and not actually a race, and it will be fixed automatically with K/N @volatile that is coming in 1.8.20

@qwwdfsad qwwdfsad closed this as completed Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug native
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@qwwdfsad @05nelsonm