From 30e8e2d182e7bd2836cfb669d28912a7b155d2b7 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Wed, 23 Oct 2024 14:38:38 +0200 Subject: [PATCH] ews: fix an ABA lockup involving EWS unsubscribe actions There is a ABA deadlock occurring: * T83/P11041 holding dbase.giant_lock * T83/P11041 waiting on EWSPlugin::subscriptionLock here: ``` f4 std::unique_lock::lock (this=this@entry=0x7f148dffbd10) at /usr/include/c++/12/bits/unique_lock.h:139 f5 std::unique_lock::unique_lock (__m=..., this=0x7f148dffbd10) at /usr/include/c++/12/bits/unique_lock.h:69 f6 gromox::EWS::EWSPlugin::event (this=0x75b860, dir=, ID=627, notification=0x7f13fc571a98) at exch/ews/ews.cpp:812 f7 ews_event_proc (dir=, table=, ID=, notification=) at exch/ews/ews.cpp:761 f8 exmdb_server::event_proc (dir=0x7f1484cc0270 "/var/lib/gromox/user/7/4", is_table=0, notify_id=627, datagram=datagram@entry=0x7f13fc571a98) at exch/exmdb/server.cpp:133 f9 notification_agent_backward_notify (remote_id=, pnotify=pnotify@entry=0x7f13fc571a70) at exch/exmdb/notification_agent.cpp:22 f10 dg_notify (notifq=...) at exch/exmdb/db_engine.cpp:574 f11 exmdb_server::write_message_v2 (dir=, cpid=cpid@entry=CP_ACP, folder_id=, pmsgctnt=0x7f1484e2eba0, outmid=outmid@entry=0x7f148dffc3a8, outcn=outcn@entry=0x7f148dffc3b0, pe_result=0x7f1484bd20ac) at exch/exmdb/message.cpp:3851 f12 exmdb_server::write_message (dir=dir@entry=0x7f1484cc0270 "/var/lib/gromox/user/7/4", cpid=cpid@entry=CP_ACP, folder_id=folder_id@entry=15137215173615419393, ctnt=ctnt@entry=0x7f1484e2eba0, e_result=e_result@entry=0x7f1484bd20ac) at exch/exmdb/message.cpp:3864 f13 exmdb_server::flush_instance (dir=0x7f1484cc0270 "/var/lib/gromox/user/7/4", instance_id=, pe_result=0x7f1484bd20ac) at exch/exmdb/instance.cpp:1518 ``` * T72/P10387 holding EWSPlugin::subscriptionLock. * T72/P10387 waiting on dbase.giant_lock here: ``` f3 std::shared_mutex::lock (this=) at /usr/include/c++/12/shared_mutex:420 f4 db_conn::lock_base_wr (this=this@entry=0x7f149a381790) at exch/exmdb/db_engine.cpp:509 f5 exmdb_server::unsubscribe_notification (dir=dir@entry=0x7f1460e6bbf0 "/var/lib/gromox/user/7/4", sub_id=, sub_id@entry=629) at /usr/include/c++/12/optional:306 f6 exmdb_client_local::unsubscribe_notification (dir=0x7f1460e6bbf0 "/var/lib/gromox/user/7/4", sub_id=629) at exch/exmdb/rpc.cpp:1681 f7 gromox::EWS::EWSPlugin::unsubscribe (this=0x75b860, key={...}) at exch/ews/ews.cpp:1104 f8 gromox::EWS::EWSPlugin::Subscription::~Subscription (this=0x7f1460964a90, __in_chrg=) at exch/ews/ews.cpp:794 ... f16 std::shared_ptr::~shared_ptr (this=0x7f149a3818a0, __in_chrg=) at /usr/include/c++/12/bits/shared_ptr.h:175 f17 gromox::EWS::EWSPlugin::unsubscribe (this=0x75b860, subscriptionKey=, username=0x7f149e59977c "user@domain.at") at exch/ews/ews.cpp:1091 f18 gromox::EWS::EWSContext::unsubscribe (this=this@entry=0x7f14630fc860, subscriptionId=...) at exch/ews/context.cpp:2204 f19 gromox::EWS::EWSContext::~EWSContext (this=this@entry=0x7f14630fc860, __in_chrg=) at exch/ews/context.cpp:342 ... f22 std::unique_ptr >::reset (__p=0x0, this=) at /usr/include/c++/12/bits/unique_ptr.h:501 f23 gromox::EWS::EWSPlugin::term (this=, ctx=) at exch/ews/ews.cpp:755 f24 operator() (ctx=, __closure=0x0) at exch/ews/ews.cpp:593 f25 _FUN () at exch/ews/ews.cpp:593 f26 hpm_processor_insert_ctx (phttp=phttp@entry=0x7f149e599338) at exch/http/hpm_processor.cpp:500 f27 http_end (ctx=ctx@entry=0x7f149e599338) at exch/http/http_parser.cpp:489 f28 http_parser_process (vcontext=0x7f149e599338) at exch/http/http_parser.cpp:2123 ``` EWSPlugin::SubManager::inner_subs does not need protection (especially not by foreign lock EWSPlugin::subscriptionLock) because it is only ever touched during creation (in ``make_submgr()``) and then again in the destructor. References: DESK-2280 DESK-2535 --- exch/ews/ews.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/exch/ews/ews.cpp b/exch/ews/ews.cpp index 0bd1a0102..63e66a6fb 100644 --- a/exch/ews/ews.cpp +++ b/exch/ews/ews.cpp @@ -790,7 +790,6 @@ EWSPlugin::SubManager::SubManager(const char *uname, const EWSPlugin &plugin) : */ EWSPlugin::SubManager::~SubManager() { - std::lock_guard ss_lock(ews.subscriptionLock); for (const auto &subKey : inner_subs) ews.unsubscribe(subKey); if(waitingContext)