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

ceph_test_rados_api_watch_notify: flush after unwatch #16402

Merged
merged 1 commit into from Jul 20, 2017

Conversation

Projects
None yet
2 participants
@liewegas
Member

liewegas commented Jul 18, 2017

After we unwatch, we need to make sure the finisher thread has flushed
its work before we tear down the test. Otherwise, the notify callback
may dereference the test object looking for a member and segfault.

Probably-Fixes: http://tracker.ceph.com/issues/20105
Signed-off-by: Sage Weil sage@redhat.com

ceph_test_rados_api_watch_notify: flush after unwatch
After we unwatch, we need to make sure the finisher thread has flushed
its work before we tear down the test.  Otherwise, the notify callback
may dereference the test object looking for a member and segfault.

Probably-Fixes: http://tracker.ceph.com/issues/20105
Signed-off-by: Sage Weil <sage@redhat.com>
@@ -283,6 +283,7 @@ TEST_F(LibRadosWatchNotify, Watch2Delete) {
ASSERT_EQ(-ENOTCONN, notify_err);
ASSERT_EQ(-ENOTCONN, rados_watch_check(ioctx, handle));
rados_unwatch2(ioctx, handle);
rados_watch_flush(cluster);

This comment has been minimized.

@tchaikov

tchaikov Jul 19, 2017

Contributor

this looks good to me. probably we can also do the cleanup in TearDown(), as the fixture is designed for this purpose. like

diff --git a/src/test/librados/watch_notify.cc b/src/test/librados/watch_notify.cc
index e28fd52711..e408b90abf 100644
--- a/src/test/librados/watch_notify.cc
+++ b/src/test/librados/watch_notify.cc
@@ -55,6 +55,10 @@ protected:
                                     void *data,
                                     size_t data_len);
   static void watch_notify2_test_errcb(void *arg, uint64_t cookie, int err);
+  void TearDown() override {
+    RadosTest::TearDown();
+    rados_watch_flush(cluster);
+  }
 };


@@ -101,6 +105,10 @@ protected:
   int notify_err = 0;

   friend class WatchNotifyTestCtx2;
+  void TearDown() override {
+    RadosTestParamPP::TearDown();
+    cluster.watch_flush();
+  }
 };

 IoCtx *notify_ioctx;
@@ -831,6 +839,8 @@ TEST_F(LibRadosWatchNotify, Watch3Timeout) {

   // re-watch
   rados_unwatch2(ioctx, handle);
+  rados_watch_flush(cluster);
+
   handle = 0;
   ASSERT_EQ(0,
            rados_watch2(ioctx, notify_oid, &handle,

@liewegas liewegas merged commit 676ea6e into ceph:master Jul 20, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@liewegas liewegas deleted the liewegas:wip-20105 branch Jul 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment