Skip to content

Data races on SelectImplementation.clauses and SelectImplementation.internalResult #3843

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
Closed
chaoren opened this issue Aug 10, 2023 · 3 comments
Closed

Data races on SelectImplementation.clauses and SelectImplementation.internalResult #3843

chaoren opened this issue Aug 10, 2023 · 3 comments
Assignees
@qwwdfsad

Comments

@chaoren
Copy link
Contributor

chaoren commented Aug 10, 2023 •
edited
Loading

Our internal TSan tests found the following data race.

Details
==================
WARNING: ThreadSanitizer: data race (pid=416)
  Write of size 4 at 0x0000d6b50bd8 by thread T4 (mutexes: write M0, write M1):
    #0 kotlinx.coroutines.selects.SelectImplementation.invoke(Ljava/lang/Throwable;)V Select.kt:754 
    #1 kotlinx.coroutines.CancellableContinuationImpl.callCancelHandler(Lkotlinx/coroutines/CancelHandler;Ljava/lang/Throwable;)V CancellableContinuationImpl.kt:249 
    #2 kotlinx.coroutines.CancellableContinuationImpl.cancel(Ljava/lang/Throwable;)Z CancellableContinuationImpl.kt:212 
    #3 kotlinx.coroutines.CancellableContinuationImpl.parentCancelled$kotlinx_coroutines_core(Ljava/lang/Throwable;)V CancellableContinuationImpl.kt:224 
    #4 kotlinx.coroutines.ChildContinuation.invoke(Ljava/lang/Throwable;)V JobSupport.kt:1450 
    #5 kotlinx.coroutines.JobSupport.notifyCancelling(Lkotlinx/coroutines/NodeList;Ljava/lang/Throwable;)V JobSupport.kt:1479 
    #6 kotlinx.coroutines.JobSupport.tryMakeCancelling(Lkotlinx/coroutines/Incomplete;Ljava/lang/Throwable;)Z JobSupport.kt:799 
    #7 kotlinx.coroutines.JobSupport.makeCancelling(Ljava/lang/Object;)Ljava/lang/Object; JobSupport.kt:759 
    #8 kotlinx.coroutines.JobSupport.cancelImpl$kotlinx_coroutines_core(Ljava/lang/Object;)Z JobSupport.kt:675 
    #9 kotlinx.coroutines.JobSupport.cancelInternal(Ljava/lang/Throwable;)V JobSupport.kt:636 
    #10 kotlinx.coroutines.JobSupport.cancel(Ljava/util/concurrent/CancellationException;)V JobSupport.kt:621 
    #11 kotlinx.coroutines.Job$DefaultImpls.cancel$default(Lkotlinx/coroutines/Job;Ljava/util/concurrent/CancellationException;ILjava/lang/Object;)V Job.kt:199 
    #12 kotlinx.coroutines.JobKt__JobKt.cancelAndJoin(Lkotlinx/coroutines/Job;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; Job.kt:511 
    #13 kotlinx.coroutines.JobKt.cancelAndJoin(Lkotlinx/coroutines/Job;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; ??:1 
    #14 kotlinx.coroutines.sync.MutexStressTest$stressUnlockCancelRaceWithSelect$1.invokeSuspend(Ljava/lang/Object;)Ljava/lang/Object; MutexStressTest.kt:113 
    #15 kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(Ljava/lang/Object;)V ContinuationImpl.kt:33 
    #16 kotlinx.coroutines.DispatchedTask.run()V DispatchedTask.kt:108 
    #17 kotlinx.coroutines.EventLoopImplBase.processNextEvent()J EventLoop.common.kt:280 
    #18 kotlinx.coroutines.BlockingCoroutine.joinBlocking()Ljava/lang/Object; Builders.kt:85 
    #19 kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Lkotlin/coroutines/CoroutineContext;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; Builders.kt:59 
    #20 kotlinx.coroutines.BuildersKt.runBlocking(Lkotlin/coroutines/CoroutineContext;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; ??:1 
    #21 kotlinx.coroutines.TestBase.runTest(Lkotlin/jvm/functions/Function1;Ljava/util/List;Lkotlin/jvm/functions/Function2;)V TestBase.kt:224 
    #22 kotlinx.coroutines.TestBase.runTest$default(Lkotlinx/coroutines/TestBase;Lkotlin/jvm/functions/Function1;Ljava/util/List;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)V TestBase.kt:216 
    #23 kotlinx.coroutines.sync.MutexStressTest.stressUnlockCancelRaceWithSelect()V MutexStressTest.kt:89 

  Previous read of size 4 at 0x0000d6b50bd8 by thread T85:
    #0 kotlinx.coroutines.selects.SelectImplementation.findClause(Ljava/lang/Object;)Lkotlinx/coroutines/selects/SelectImplementation$ClauseData; Select.kt:656 
    #1 kotlinx.coroutines.selects.SelectImplementation.trySelectInternal(Ljava/lang/Object;Ljava/lang/Object;)I Select.kt:615 
    #2 kotlinx.coroutines.selects.SelectImplementation.trySelect(Ljava/lang/Object;Ljava/lang/Object;)Z Select.kt:600 
    #3 kotlinx.coroutines.sync.MutexImpl$SelectInstanceWithOwner.trySelect(Ljava/lang/Object;Ljava/lang/Object;)Z Mutex.kt:285 
    #4 kotlinx.coroutines.sync.SemaphoreImpl.tryResumeAcquire(Ljava/lang/Object;)Z Semaphore.kt:354 
    #5 kotlinx.coroutines.sync.SemaphoreImpl.tryResumeNextFromQueue()Z Semaphore.kt:340 
    #6 kotlinx.coroutines.sync.SemaphoreImpl.release()V Semaphore.kt:265 
    #7 kotlinx.coroutines.sync.MutexImpl.unlock(Ljava/lang/Object;)V Mutex.kt:222 
    #8 kotlinx.coroutines.sync.Mutex$DefaultImpls.unlock$default(Lkotlinx/coroutines/sync/Mutex;Ljava/lang/Object;ILjava/lang/Object;)V Mutex.kt:98 
    #9 kotlinx.coroutines.sync.MutexStressTest$stressUnlockCancelRaceWithSelect$1$1$1$job2$1.invokeSuspend(Ljava/lang/Object;)Ljava/lang/Object; MutexStressTest.kt:109
@chaoren chaoren added the bug label Aug 10, 2023
@chaoren
Copy link
Contributor Author

chaoren commented Aug 10, 2023 •
edited
Loading

Also a data race on SelectImplementation.internalResult (after putting @Volatile on SelectImplementation.clauses).

Details
==================
WARNING: ThreadSanitizer: data race (pid=4295)
  Write of size 4 at 0x0000cd249cc0 by thread T4:
    #0 kotlinx.coroutines.selects.SelectImplementation.invoke(Ljava/lang/Throwable;)V Select.kt:753 
    #1 kotlinx.coroutines.CancellableContinuationImpl.callCancelHandler(Lkotlinx/coroutines/CancelHandler;Ljava/lang/Throwable;)V CancellableContinuationImpl.kt:249 
    #2 kotlinx.coroutines.CancellableContinuationImpl.cancel(Ljava/lang/Throwable;)Z CancellableContinuationImpl.kt:212 
    #3 kotlinx.coroutines.CancellableContinuationImpl.parentCancelled$kotlinx_coroutines_core(Ljava/lang/Throwable;)V CancellableContinuationImpl.kt:224 
    #4 kotlinx.coroutines.ChildContinuation.invoke(Ljava/lang/Throwable;)V JobSupport.kt:1450 
    #5 kotlinx.coroutines.JobSupport.notifyCancelling(Lkotlinx/coroutines/NodeList;Ljava/lang/Throwable;)V JobSupport.kt:1479 
    #6 kotlinx.coroutines.JobSupport.tryMakeCancelling(Lkotlinx/coroutines/Incomplete;Ljava/lang/Throwable;)Z JobSupport.kt:799 
    #7 kotlinx.coroutines.JobSupport.makeCancelling(Ljava/lang/Object;)Ljava/lang/Object; JobSupport.kt:759 
    #8 kotlinx.coroutines.JobSupport.cancelImpl$kotlinx_coroutines_core(Ljava/lang/Object;)Z JobSupport.kt:675 
    #9 kotlinx.coroutines.JobSupport.cancelInternal(Ljava/lang/Throwable;)V JobSupport.kt:636 
    #10 kotlinx.coroutines.JobSupport.cancel(Ljava/util/concurrent/CancellationException;)V JobSupport.kt:621 
    #11 kotlinx.coroutines.Job$DefaultImpls.cancel$default(Lkotlinx/coroutines/Job;Ljava/util/concurrent/CancellationException;ILjava/lang/Object;)V Job.kt:199 
    #12 kotlinx.coroutines.JobKt__JobKt.cancelAndJoin(Lkotlinx/coroutines/Job;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; Job.kt:511 
    #13 kotlinx.coroutines.JobKt.cancelAndJoin(Lkotlinx/coroutines/Job;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; ??:1 
    #14 kotlinx.coroutines.sync.MutexStressTest$stressUnlockCancelRaceWithSelect$1.invokeSuspend(Ljava/lang/Object;)Ljava/lang/Object; MutexStressTest.kt:113 
    #15 kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(Ljava/lang/Object;)V ContinuationImpl.kt:33 
    #16 kotlinx.coroutines.DispatchedTask.run()V DispatchedTask.kt:108 
    #17 kotlinx.coroutines.EventLoopImplBase.processNextEvent()J EventLoop.common.kt:280 
    #18 kotlinx.coroutines.BlockingCoroutine.joinBlocking()Ljava/lang/Object; Builders.kt:85 
    #19 kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Lkotlin/coroutines/CoroutineContext;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; Builders.kt:59 
    #20 kotlinx.coroutines.BuildersKt.runBlocking(Lkotlin/coroutines/CoroutineContext;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; ??:1 
    #21 kotlinx.coroutines.TestBase.runTest(Lkotlin/jvm/functions/Function1;Ljava/util/List;Lkotlin/jvm/functions/Function2;)V TestBase.kt:224 
    #22 kotlinx.coroutines.TestBase.runTest$default(Lkotlinx/coroutines/TestBase;Lkotlin/jvm/functions/Function1;Ljava/util/List;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)V TestBase.kt:216 
    #23 kotlinx.coroutines.sync.MutexStressTest.stressUnlockCancelRaceWithSelect()V MutexStressTest.kt:89 

  Previous write of size 4 at 0x0000cd249cc0 by thread T18:
    #0 kotlinx.coroutines.selects.SelectImplementation.trySelectInternal(Ljava/lang/Object;Ljava/lang/Object;)I Select.kt:622 
    #1 kotlinx.coroutines.selects.SelectImplementation.trySelect(Ljava/lang/Object;Ljava/lang/Object;)Z Select.kt:600 
    #2 kotlinx.coroutines.sync.MutexImpl$SelectInstanceWithOwner.trySelect(Ljava/lang/Object;Ljava/lang/Object;)Z Mutex.kt:285 
    #3 kotlinx.coroutines.sync.SemaphoreImpl.tryResumeAcquire(Ljava/lang/Object;)Z Semaphore.kt:354 
    #4 kotlinx.coroutines.sync.SemaphoreImpl.tryResumeNextFromQueue()Z Semaphore.kt:340 
    #5 kotlinx.coroutines.sync.SemaphoreImpl.release()V Semaphore.kt:265 
    #6 kotlinx.coroutines.sync.MutexImpl.unlock(Ljava/lang/Object;)V Mutex.kt:222 
    #7 kotlinx.coroutines.sync.Mutex$DefaultImpls.unlock$default(Lkotlinx/coroutines/sync/Mutex;Ljava/lang/Object;ILjava/lang/Object;)V Mutex.kt:98 
    #8 kotlinx.coroutines.sync.MutexStressTest$stressUnlockCancelRaceWithSelect$1$1$1$job2$1.invokeSuspend(Ljava/lang/Object;)Ljava/lang/Object; MutexStressTest.kt:109

@chaoren
Copy link
Contributor Author

chaoren commented Aug 10, 2023

@Volatile fixes the issue for both.

@chaoren chaoren changed the title Data race on SelectImplementation.clauses Data races on SelectImplementation.clauses and SelectImplementation.internalResult Aug 11, 2023
@qwwdfsad qwwdfsad self-assigned this Aug 31, 2023
qwwdfsad added a commit that referenced this issue Aug 31, 2023
@qwwdfsad
Explain benign data-race on SelectImplementation.clauses and mark it …
3e5acfd 
…with @BenignDataRace

Fixes #3843
qwwdfsad added a commit that referenced this issue Aug 31, 2023
@qwwdfsad
Explain benign data-race on SelectImplementation.internalResult and m…
6d91be8 
…ark it with @BenignDataRace

Fixes #3843
@qwwdfsad
Copy link
Collaborator

The same goes for this one: both are benign, both are explained and marked with the annotation in #3873

@qwwdfsad qwwdfsad removed the bug label Aug 31, 2023
igoriakovlev pushed a commit that referenced this issue Nov 2, 2023
@qwwdfsad @igoriakovlev
Introduce the notion of @BeningDataRace (#3873)
6f214c8 
* Mark BufferedChannelIterator.continuation as @BeningDataRace to address potential UB and OoTA on K/N
* Explain benign data-race on SelectImplementation.clauses and mark it with @BenignDataRace
* Explain benign data-race on SelectImplementation.internalResult and mark it with @BenignDataRace


Fixes #3834
Fixes #3843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@qwwdfsad qwwdfsad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@qwwdfsad @chaoren