From 3bcb00ab5855c6af4dc2f3e589c56b82d381441d Mon Sep 17 00:00:00 2001 From: heck Date: Tue, 20 Dec 2022 19:52:59 +0100 Subject: [PATCH] Fix: call register_sync_callbacks() ONLY in sync thread. No other session/thread will have sync callbacks registered. This is essentially reverting a change that has been done for 3.0, but i think it was wrong --- src/Adapter.cc | 15 +++------------ src/Adapter.hxx | 22 +++++++++++++++------- test/test_leave_device_group.cc | 31 ++++++++++++++++--------------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/Adapter.cc b/src/Adapter.cc index 4486616..aa8b9e8 100644 --- a/src/Adapter.cc +++ b/src/Adapter.cc @@ -150,7 +150,6 @@ namespace pEp { void Session::refresh() { std::lock_guard lock(mut); - release(); // Switch to mode "Sync" ensures the sync thread to be shutdown if (_sync_mode == SyncModes::Sync) { @@ -172,17 +171,9 @@ namespace pEp { ::PEP_SESSION session_{ nullptr }; ::PEP_STATUS status = ::init(&session_, _messageToSend, _inject_action, _ensure_passphrase); throw_status(status); - status = ::register_sync_callbacks( - session_, - nullptr, - _notifyHandshake, - _retrieve_next_sync_event); - if (status != PEP_STATUS_OK) { - pEpLog("libpEpAdapter: WARNING - session is initialized but without sync/callbacks. " - "This is normal if there are no own identities yet. Call session.init() again to " - "re-initialize the session after creating an own identity."); - } - // store + + // replace "atomically" + release(); _session = SessionPtr{ session_, ::release }; } diff --git a/src/Adapter.hxx b/src/Adapter.hxx index 5d629eb..8dac021 100644 --- a/src/Adapter.hxx +++ b/src/Adapter.hxx @@ -46,15 +46,23 @@ namespace pEp { _startup(obj); } - pEpLog("creating session for the sync thread"); // 2. Create session for the sync thread - // 3. register_sync_callbacks() (in session.initialize()) + pEpLog("creating session for the sync thread"); + session.initialize( + rhs->_sync_mode, + rhs->_adapter_manages_sync_thread, + rhs->_messageToSend, + rhs->_notifyHandshake); + try { - session.initialize( - rhs->_sync_mode, - rhs->_adapter_manages_sync_thread, - rhs->_messageToSend, - rhs->_notifyHandshake); + // 3. register_sync_callbacks() + ::PEP_STATUS status = ::register_sync_callbacks( + session(), + nullptr, + session._notifyHandshake, + _retrieve_next_sync_event); + throw_status(status); + register_done.store(true); } catch (...) { _ex = std::current_exception(); diff --git a/test/test_leave_device_group.cc b/test/test_leave_device_group.cc index 532fbcd..bbc3903 100644 --- a/test/test_leave_device_group.cc +++ b/test/test_leave_device_group.cc @@ -29,8 +29,8 @@ vector<::sync_handshake_signal> expected_notification = { SYNC_NOTIFY_IN_GROUP, ::PEP_STATUS test_messageToSend(::message* _msg) { - static auto actual = expected_msg.begin(); + static auto actual = expected_msg.begin(); Test::Message msg = Test::make_message(_msg); string text = Test::make_pEp_msg(msg); cerr << "expecting: " << *actual << endl; @@ -54,35 +54,38 @@ vector<::sync_handshake_signal> expected_notification = { SYNC_NOTIFY_IN_GROUP, int main(int argc, char** argv) { + pEp::Adapter::pEpLog::set_enabled(true); Test::setup(argc, argv); - Adapter::session.initialize(pEp::Adapter::SyncModes::Async, false); + pEpLog(getenv("HOME")); + int n; + std::cin >> n; + Adapter::session.initialize(Adapter::SyncModes::Async, false); // set up two own identites for sync - passphrase_cache.add("erwin"); passphrase_cache.add("bob"); - const char* bob_filename = ENGINE_TEST - "/test_keys/bob-primary-with-password-bob-subkey-without.pgp"; - const char* bob_fpr = "5C76378A62B04CF3F41BEC8D4940FC9FA1878736"; + std::string bob_filename = Test::get_resource_abs( + "bob-primary-with-password-bob-subkey-without.pgp"); + std::string bob_fpr = "5C76378A62B04CF3F41BEC8D4940FC9FA1878736"; - const char* erwin_filename = ENGINE_TEST "/test_keys/erwin_normal_encrypted.pgp"; - const char* erwin_fpr = "CBA968BC01FCEB89F04CCF155C5E9E3F0420A570"; + std::string erwin_filename = Test::get_resource_abs("erwin_normal_encrypted.pgp"); + std::string erwin_fpr = "CBA968BC01FCEB89F04CCF155C5E9E3F0420A570"; Test::import_key_from_file(bob_filename); Test::import_key_from_file(erwin_filename); Test::Identity bob = Test::make_identity( - ::new_identity("bob@example.org", bob_fpr, "BOB", "Bob Dog")); - PEP_STATUS status = ::set_own_key(Adapter::session(), bob.get(), bob_fpr); + ::new_identity("bob@example.org", bob_fpr.c_str(), "BOB", "Bob Dog")); + PEP_STATUS status = ::set_own_key(Adapter::session(), bob.get(), bob_fpr.c_str()); assert(status == PEP_STATUS_OK); status = ::enable_identity_for_sync(Adapter::session(), bob.get()); assert(status == PEP_STATUS_OK); Test::Identity erwin = Test::make_identity( - ::new_identity("erwin@example.org", erwin_fpr, "BOB", "Bob is Erwin")); - status = ::set_own_key(Adapter::session(), erwin.get(), erwin_fpr); + ::new_identity("erwin@example.org", erwin_fpr.c_str(), "BOB", "Bob is Erwin")); + status = ::set_own_key(Adapter::session(), erwin.get(), erwin_fpr.c_str()); assert(status == PEP_STATUS_OK); status = ::enable_identity_for_sync(Adapter::session(), erwin.get()); @@ -99,10 +102,8 @@ int main(int argc, char** argv) Adapter::start_sync(); // leave device group - - status = ::leave_device_group(Adapter::session()); - + throw_status(status); // wait for sync shutdown and release first session Test::join_sync_thread();