Rename CancellationFlag to AtomicFlag.

base/synchronization/atomic_flag_unittest.cc

Index: base/synchronization/atomic_flag_unittest.cc
diff --git a/base/synchronization/atomic_flag_unittest.cc b/base/synchronization/atomic_flag_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c881d5b40c71f01ba7d2cc79c68ee33fe9c68d5a
--- /dev/null
+++ b/base/synchronization/atomic_flag_unittest.cc
@@ -0,0 +1,83 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Tests of AtomicFlag class.
+
+#include "base/synchronization/atomic_flag.h"
+
+#include "base/bind.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+namespace {
+
+void CancelHelper(AtomicFlag* flag) {

[danakj, 2016/07/20 19:21:01]

rename to SetFlag or something?

[fdoray, 2016/07/21 13:08:33]

Done.

+  ASSERT_TRUE(flag);
+#if GTEST_HAS_DEATH_TEST
+  ASSERT_DEBUG_DEATH(flag->Set(), "");

[fdoray, 2016/07/20 16:24:11]

TODO: Use EXPECT_DCHECK_DEATH once https://codereview.chromium.org/2162053006/
lands.

+#endif
+}
+
+void ExpectIsNotSet(AtomicFlag* flag) {
+  ASSERT_TRUE(flag);
+  EXPECT_FALSE(flag->IsSet());
+}
+
+void ExpectIsSet(AtomicFlag* flag) {
+  ASSERT_TRUE(flag);
+  EXPECT_TRUE(flag->IsSet());
+}
+
+}  // namespace
+
+TEST(AtomicFlagTest, SimpleSingleThreadedTest) {
+  AtomicFlag flag;
+  ASSERT_FALSE(flag.IsSet());
+  flag.Set();
+  ASSERT_TRUE(flag.IsSet());
+}
+
+TEST(AtomicFlagTest, DoubleSetTest) {
+  AtomicFlag flag;
+  ASSERT_FALSE(flag.IsSet());
+  flag.Set();
+  ASSERT_TRUE(flag.IsSet());
+  flag.Set();
+  ASSERT_TRUE(flag.IsSet());
+}
+
+TEST(AtomicFlagTest, ReadFromDifferentThread) {
+  AtomicFlag flag;
+
+  {
+    Thread t("AtomicFlagTest.ReadFromDifferentThread.ExpectIsNotSetThread");
+    ASSERT_TRUE(t.Start());
+    t.task_runner()->PostTask(FROM_HERE, Bind(&ExpectIsNotSet, &flag));
+  }
+
+  flag.Set();
+
+  {
+    Thread t("AtomicFlagTest.ReadFromDifferentThread.ExpectIsSetThread");
+    ASSERT_TRUE(t.Start());
+    t.task_runner()->PostTask(FROM_HERE, Bind(&ExpectIsSet, &flag));

[danakj, 2016/07/20 19:21:01]

post task is a memory barrier so this test is kinda silly, it's not testing
anything, it would pass even without any atomics right?

you could make this test better by posting the task before calling Set, and have
it spin until it sees the flag update.

And maybe use atomics in the test to tell the other thread when it should expect
to see it updated.

[fdoray, 2016/07/21 13:08:33]

sgtm
If I use atomics in the test, they'll be accompanied with memory barriers and
we'll have the same problem as we have now.

[gab, 2016/07/21 13:27:12]

True, then maybe sleep here for say 10ms before calling Set() to increase
likelihood of Set() being called after the busy wait started? (with the yield
suggested in the loop it should work fine)

[fdoray, 2016/07/21 14:05:30]

sgtm.

I'm still a little bit worried that Sleep, YieldCurrentThread and the join in
base::Thread's destructor may act as memory barriers, but I don't think we can
do better.

[gab, 2016/07/21 14:16:04]

I doubt Sleep and Yield result in memory barriers. Join definitely does but I
think the barrier is an rfence (Release) on the thread going away and an lfence
(Acquire) on the Join side (anything else doesn't make much sense IMO) -- i.e.,
I don't think calling Join from thread A on thread B changes anything for thread
B (which is the one we're testing atomics on so it should be okay).

[fdoray, 2016/07/21 15:50:02]

Shouldn't the memory be synchronized when the thread is descheduled? That way,
the thread will be able to see what it wrote in memory when it is re-scheduled
on another core.

[gab, 2016/07/21 15:56:42]

I don't think memory *has* to be synchronized when the thread is descheduled.
The kernel will have to if it wants to reschedule on another core, but there may
very well be heuristics that only synchronize when such a move occurs instead of
always.

The only guarantee of memory order is that a single-threaded program shouldn't
experience a difference. That doesn't mean it has to be synchronized every time
it's descheduled IMO.

+  }
+}
+
+TEST(AtomicFlagTest, SetOnDifferentThreadDeathTest) {
+  // Checks that Set() can't be called from any other thread.
+  // AtomicFlag should die on a DCHECK if Set() is called from
+  // other thread.
+  ::testing::FLAGS_gtest_death_test_style = "threadsafe";
+  Thread t("AtomicFlagTest.SetOnDifferentThreadDeathTest");
+  ASSERT_TRUE(t.Start());
+
+  AtomicFlag flag;
+  t.task_runner()->PostTask(FROM_HERE, Bind(&CancelHelper, &flag));
+}
+
+}  // namespace base