From 8585803c89c0d2480e7433bf199347b48c9bc7c7 Mon Sep 17 00:00:00 2001 From: Kamil Cudnik Date: Thu, 25 Mar 2021 23:11:35 +0100 Subject: [PATCH] [sairedis] Unlock api mutex for communication mode (#812) This will remove possible deadlock when notification arrives during communication channel destructor thread join. --- lib/inc/SaiInternal.h | 1 + lib/src/Sai.cpp | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/inc/SaiInternal.h b/lib/inc/SaiInternal.h index 2881beec1f8f..5ab4da121baa 100644 --- a/lib/inc/SaiInternal.h +++ b/lib/inc/SaiInternal.h @@ -1,3 +1,4 @@ #pragma once #define MUTEX() std::lock_guard _lock(m_apimutex) +#define MUTEX_UNLOCK() m_apimutex.unlock() diff --git a/lib/src/Sai.cpp b/lib/src/Sai.cpp index 489741c3c2c0..1bb9fbff1eac 100644 --- a/lib/src/Sai.cpp +++ b/lib/src/Sai.cpp @@ -196,6 +196,25 @@ sai_status_t Sai::set( if (RedisRemoteSaiInterface::isRedisAttribute(objectType, attr)) { + if (attr->id == SAI_REDIS_SWITCH_ATTR_REDIS_COMMUNICATION_MODE) + { + // Since communication mode destroys current channel and creates + // new one, it may happen, that during this SET api execution when + // api mutex is acquired, channel destructor will be blocking on + // thread->join() and channel thread will start processing + // incoming notification. That notification will be synchronized + // with api mutex and will cause deadlock, so to mitigate this + // scenario we will unlock api mutex here. + // + // This is not the perfect, but assuming that communication mode is + // changed in single thread and before switch create then we should + // not hit race condition. + + SWSS_LOG_NOTICE("unlocking api mutex for communication mode"); + + MUTEX_UNLOCK(); + } + // skip metadata if attribute is redis extension attribute // TODO this is setting on all contexts, but maybe we want one specific?