2482

Committed 11 Jul 2024 01:03AM UTC coverage: 90.971%. First build

Build # 2482

Build Type

push

Evergreen

Committed by

web-flow

Commit Message

Fix data races in the connection state changes tests (#7871)

The assignment to token1 on the main thread and the read of it on the sync
worker thread didn't have any intervening synchronization and in theory the
callback could read it before it's actually written. Fixing this requires
adding some locking.

Conversely, listener2 doesn't actually need to be atomic since the second
callback should only ever be invokved synchronously inside log_out(), and if
it's called at some other time that's a bug.

It doesn't matter here, but `listener1_call_cnt = listener1_call_cnt + 1` is a
nonatomic increment that will drop updates if it happens on multiple threads at
once, while `++` will not.

Run Details

102278 of 180462 branches covered (56.68%)

8 of 9 new or added lines in 1 file covered. (88.89%)

215177 of 236534 relevant lines covered (90.97%)

5615481.07 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

96.2

/test/object-store/sync/session/connection_change_notifications.cpp

////////////////////////////////////////////////////////////////////////////
//
// Copyright 2018 Realm Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

#include <util/event_loop.hpp>
#include <util/test_utils.hpp>
#include <util/sync/session_util.hpp>

using namespace realm;
using namespace realm::util;

TEST_CASE("sync: Connection state changes", "[sync][session][connection change]") {
    if (!EventLoop::has_implementation())
        return;

    TestSyncManager tsm;
    auto user = tsm.fake_user();

    SECTION("register connection change listener") {
        auto session = sync_session(
            user, "/connection-state-changes-1", [](auto, auto) {}, SyncSessionStopPolicy::AfterChangesUploaded);

        EventLoop::main().run_until([&] {
            return sessions_are_active(*session);
        });
        EventLoop::main().run_until([&] {
            return sessions_are_connected(*session);
        });

        std::atomic<bool> listener_called(false);
        session->register_connection_change_callback([&](SyncSession::ConnectionState, SyncSession::ConnectionState) {
            listener_called = true;
        });

        user->log_out();
        EventLoop::main().run_until([&] {
            return sessions_are_disconnected(*session);
        });
        REQUIRE(listener_called == true);
    }

    SECTION("unregister connection change listener") {
        auto session = sync_session(
            user, "/connection-state-changes-2", [](auto, auto) {}, SyncSessionStopPolicy::AfterChangesUploaded);

        EventLoop::main().run_until([&] {
            return sessions_are_active(*session);
        });
        EventLoop::main().run_until([&] {
            return sessions_are_connected(*session);
        });

        std::atomic<size_t> listener1_call_cnt(0);
        std::atomic<bool> listener2_called(false);

        auto token1 = session->register_connection_change_callback(
            [&](SyncSession::ConnectionState, SyncSession::ConnectionState) {
                ++listener1_call_cnt;
            });
        session->unregister_connection_change_callback(token1);
        // One call may have been in progress when unregistered
        REQUIRE(listener1_call_cnt <= 1);
        size_t listener1_called = listener1_call_cnt;

        session->register_connection_change_callback([&](SyncSession::ConnectionState, SyncSession::ConnectionState) {
            listener2_called = true;
        });
        user->log_out();
        REQUIRE(sessions_are_disconnected(*session));
        // ensure callback 1 was not called anymore
        REQUIRE(listener1_call_cnt == listener1_called);
        REQUIRE(listener2_called);
    }

    SECTION("unregister connection change listener during callback") {
        int listener1_call_cnt = 0;
        auto session = sync_session(
            user, "/connection-state-changes-3", [](auto, auto) {}, SyncSessionStopPolicy::AfterChangesUploaded);
        std::mutex mutex;
        std::unique_lock lock(mutex);
        uint64_t token1 = session->register_connection_change_callback(
            [&](SyncSession::ConnectionState, SyncSession::ConnectionState) {
                std::lock_guard lock(mutex);
                ++listener1_call_cnt;
                session->unregister_connection_change_callback(token1);
            });
        lock.unlock();

        EventLoop::main().run_until([&] {
            return sessions_are_active(*session);
        });
        EventLoop::main().run_until([&] {
            return sessions_are_connected(*session);
        });

        bool listener2_called = false;
        session->register_connection_change_callback([&](SyncSession::ConnectionState, SyncSession::ConnectionState) {
            listener2_called = true;
        });

        user->log_out();
        REQUIRE(sessions_are_disconnected(*session));
        REQUIRE(listener1_call_cnt == 1); // Only called once before unregister
        REQUIRE(listener2_called);
    }
}

1	////////////////////////////////////////////////////////////////////////////
2	//
3	// Copyright 2018 Realm Inc.
4	//
5	// Licensed under the Apache License, Version 2.0 (the "License");
6	// you may not use this file except in compliance with the License.
7	// You may obtain a copy of the License at
8	//
9	// http://www.apache.org/licenses/LICENSE-2.0
10	//
11	// Unless required by applicable law or agreed to in writing, software
12	// distributed under the License is distributed on an "AS IS" BASIS,
13	// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14	// See the License for the specific language governing permissions and
15	// limitations under the License.
16	//
17	////////////////////////////////////////////////////////////////////////////
18
19	#include <util/event_loop.hpp>
20	#include <util/test_utils.hpp>
21	#include <util/sync/session_util.hpp>
22
23	using namespace realm;
24	using namespace realm::util;
25
26	TEST_CASE("sync: Connection state changes", "[sync][session][connection change]") {	6✔
27	if (!EventLoop::has_implementation())	6✔
28	return;	×
29
30	TestSyncManager tsm;	6✔
31	auto user = tsm.fake_user();	6✔
32
33	SECTION("register connection change listener") {	6✔
34	auto session = sync_session(	2✔
35	user, "/connection-state-changes-1", [](auto, auto) {}, SyncSessionStopPolicy::AfterChangesUploaded);	2✔
36
37	EventLoop::main().run_until([&] {	3✔
38	return sessions_are_active(*session);	3✔
39	});	3✔
40	EventLoop::main().run_until([&] {	2,223✔
41	return sessions_are_connected(*session);	2,223✔
42	});	2,223✔
43
44	std::atomic<bool> listener_called(false);	2✔
45	session->register_connection_change_callback([&](SyncSession::ConnectionState, SyncSession::ConnectionState) {	2✔
46	listener_called = true;	2✔
47	});	2✔
48
49	user->log_out();	2✔
50	EventLoop::main().run_until([&] {	3✔
51	return sessions_are_disconnected(*session);	3✔
52	});	3✔
53	REQUIRE(listener_called == true);	2!
54	}	2✔
55
56	SECTION("unregister connection change listener") {	6✔
57	auto session = sync_session(	2✔
58	user, "/connection-state-changes-2", [](auto, auto) {}, SyncSessionStopPolicy::AfterChangesUploaded);	2✔
59
60	EventLoop::main().run_until([&] {	3✔
61	return sessions_are_active(*session);	3✔
62	});	3✔
63	EventLoop::main().run_until([&] {	2,000✔
64	return sessions_are_connected(*session);	2,000✔
65	});	2,000✔
66
67	std::atomic<size_t> listener1_call_cnt(0);	2✔
68	std::atomic<bool> listener2_called(false);	2✔
69
70	auto token1 = session->register_connection_change_callback(	2✔
71	[&](SyncSession::ConnectionState, SyncSession::ConnectionState) {	2✔
NEW 72	++listener1_call_cnt;	×
73	});	×
74	session->unregister_connection_change_callback(token1);	2✔
75	// One call may have been in progress when unregistered
76	REQUIRE(listener1_call_cnt <= 1);	2!
77	size_t listener1_called = listener1_call_cnt;	2✔
78
79	session->register_connection_change_callback([&](SyncSession::ConnectionState, SyncSession::ConnectionState) {	2✔
80	listener2_called = true;	2✔
81	});	2✔
82	user->log_out();	2✔
83	REQUIRE(sessions_are_disconnected(*session));	2!
84	// ensure callback 1 was not called anymore
85	REQUIRE(listener1_call_cnt == listener1_called);	2!
86	REQUIRE(listener2_called);	2!
87	}	2✔
88
89	SECTION("unregister connection change listener during callback") {	6✔
90	int listener1_call_cnt = 0;	2✔
91	auto session = sync_session(	2✔
92	user, "/connection-state-changes-3", [](auto, auto) {}, SyncSessionStopPolicy::AfterChangesUploaded);	2✔
93	std::mutex mutex;	2✔
94	std::unique_lock lock(mutex);	2✔
95	uint64_t token1 = session->register_connection_change_callback(	2✔
96	[&](SyncSession::ConnectionState, SyncSession::ConnectionState) {	2✔
97	std::lock_guard lock(mutex);	2✔
98	++listener1_call_cnt;	2✔
99	session->unregister_connection_change_callback(token1);	2✔
100	});	2✔
101	lock.unlock();	2✔
102
103	EventLoop::main().run_until([&] {	3✔
104	return sessions_are_active(*session);	3✔
105	});	3✔
106	EventLoop::main().run_until([&] {	1,874✔
107	return sessions_are_connected(*session);	1,874✔
108	});	1,874✔
109
110	bool listener2_called = false;	2✔
111	session->register_connection_change_callback([&](SyncSession::ConnectionState, SyncSession::ConnectionState) {	2✔
112	listener2_called = true;	2✔
113	});	2✔
114
115	user->log_out();	2✔
116	REQUIRE(sessions_are_disconnected(*session));	2!
117	REQUIRE(listener1_call_cnt == 1); // Only called once before unregister	2!
118	REQUIRE(listener2_called);	2!
119	}	2✔
120	}	6✔

realm / realm-core / 2482

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous