Info

Consider a cancelation hierarchy as follows: CancelationSource first; CancelationSource second(first.token()); Now imagine some thread, thread A, calls first.cancel(); Around the same time, another thread B reads the cancellation states: if (second.isCanceled()) if (first.isCancelled()) else... Based on my understanding of memory ordering, the following could happen: When thread A cancels the first source, it marks the cancelation state for that source as "canceled" and then emplaces its cancelation promise. Because the second source is in a hierarchy with the first source, fulfilling this promise causes this continuation to run inline on thread A, which marks the cancelation state for the second source as "canceled." However, because CancelationState::isCanceled uses loadRelaxed, it is possible that thread B reads second.isCanceled() == true and first.isCanceled == false; even though the stores are ordered first --> second on thread A, the relaxed load on thread B won't force any synchronization with the system. It is permissible in the memory model for thread B to read second.isCanceled without being synchronized with all the things that happened before the second source was cancelled in thread A. (The best explanation of this for me was the last example, "Mixing Memory Models" on this page: https://gcc.gnu.org/wiki/Atomic/GCCMM/AtomicSync) Tenant migrations and resharding code wants to rely on the ordering of cancelation sources in a hierarchy to be consistently visible to other threads; I think this goal makes sense and we should forbid reading this inconsistent state. Thanks to max.hirschhorn for finding this and working through the memory model examples with me. Acceptance Criteria: isCanceled() uses load instead of loadRelaxed.

Top User Comments

JIRAUSER1259052 commented on Wed, 6 Oct 2021 18:54:01 +0000: Updating the fixversion since branching activities occurred yesterday. This ticket will be in rc0 when it’s been triggered. For more active release information, please keep an eye on #server-release. Thank you! xgen-internal-githook commented on Wed, 23 Jun 2021 16:15:59 +0000: Author: {'name': 'Alex Li', 'email': 'alex.li@mongodb.com'} Message: SERVER-55949 CancellationState::isCanceled and isCancelable use load instead of loadRelaxed (cherry picked from commit 7916b72cd1082330e05d5fd052e099f3387ca513) Branch: v5.0 https://github.com/mongodb/mongo/commit/1f68be5545592b9cb7ee8593422760bad714101e xgen-internal-githook commented on Thu, 17 Jun 2021 21:57:50 +0000: Author: {'name': 'Alex Li', 'email': 'alex.li@mongodb.com'} Message: SERVER-55949 CancellationState::isCanceled and isCancelable use load instead of loadRelaxed Branch: master https://github.com/mongodb/mongo/commit/7916b72cd1082330e05d5fd052e099f3387ca513 billy.donahue commented on Tue, 18 May 2021 16:42:08 +0000: Would this problem also apply to the loadRelaxed() call in isCancelable() ? JIRAUSER1253452 commented on Mon, 17 May 2021 16:35:05 +0000: CR - https://mongodbcr.appspot.com/788920003/

Steps to Reproduce