From de60723d031954c078ac5532f307bf142871c1d4 Mon Sep 17 00:00:00 2001 From: xuxiaoqing Date: Mon, 3 Jun 2024 16:31:52 +0800 Subject: [PATCH] fix: fix deadlock issues in DFILE_ON_CLEAR_POLICY_FILE_LIST event Signed-off-by: xuxiaoqing --- sdk/libsoftbus_client_map | 1 + .../src/client_trans_session_callback.c | 5 +- .../src/client_trans_session_manager.c | 14 +-- .../common/include/client_trans_udp_manager.h | 3 + .../udp/common/src/client_trans_udp_manager.c | 25 +++++ .../udp/file/src/client_trans_file.c | 100 ++++++++++++------ tests/sdk/transmission/session/BUILD.gn | 1 + .../udp/file/trans_sdk_file_test.cpp | 10 -- 8 files changed, 106 insertions(+), 53 deletions(-) diff --git a/sdk/libsoftbus_client_map b/sdk/libsoftbus_client_map index 1803048b9d..f6764533ed 100644 --- a/sdk/libsoftbus_client_map +++ b/sdk/libsoftbus_client_map @@ -509,6 +509,7 @@ "TransRegistDirSchema"; "TransUnRegistDirSchema"; "GetMacInfoBySession"; + "TransSetUdpChanelSessionId"; extern "C++" { OHOS::StreamAdaptor*; Communication::SoftBus*; diff --git a/sdk/transmission/session/src/client_trans_session_callback.c b/sdk/transmission/session/src/client_trans_session_callback.c index 88486e9e81..cf499ece5b 100644 --- a/sdk/transmission/session/src/client_trans_session_callback.c +++ b/sdk/transmission/session/src/client_trans_session_callback.c @@ -20,6 +20,7 @@ #include "anonymizer.h" #include "client_trans_proxy_manager.h" #include "client_trans_session_manager.h" +#include "client_trans_udp_manager.h" #include "session_set_timer.h" #include "softbus_adapter_mem.h" #include "softbus_def.h" @@ -288,7 +289,9 @@ NO_SANITIZE("cfi") int32_t TransOnSessionOpened(const char *sessionName, const C TRANS_LOGE(TRANS_SDK, "accept session failed, ret=%{public}d", ret); return ret; } - + if (channel->channelType == CHANNEL_TYPE_UDP && channel->businessType == BUSINESS_TYPE_FILE) { + TransSetUdpChanelSessionId(channel->channelId, sessionId); + } int id = SetTimer("OnSessionOpened", DFX_TIMERS_S); if (sessionCallback.isSocketListener) { ret = HandleOnBindSuccess(sessionId, sessionCallback, channel->isServer); diff --git a/sdk/transmission/session/src/client_trans_session_manager.c b/sdk/transmission/session/src/client_trans_session_manager.c index 91b38a46be..debef86dc2 100644 --- a/sdk/transmission/session/src/client_trans_session_manager.c +++ b/sdk/transmission/session/src/client_trans_session_manager.c @@ -1358,13 +1358,6 @@ static bool ClientTransCheckHmlIp(const char *ip) // determine connection type based on IP, delete session when connection type and parameter connType are consistent static bool ClientTransCheckNeedDel(SessionInfo *sessionNode, int32_t routeType, int32_t connType) { - /* - * When the channel type is UDP and the business type is file, the SessionNode is not cleared to ensure that the - * FileEvent can be reported when the link is disconnected abnormally. - */ - if (sessionNode->channelType == CHANNEL_TYPE_UDP && sessionNode->businessType == BUSINESS_TYPE_FILE) { - return false; - } if (connType == TRANS_CONN_ALL) { if (routeType != ROUTE_TYPE_ALL && sessionNode->routeType != routeType) { return false; @@ -1427,6 +1420,13 @@ static void DestroyClientSessionByNetworkId(const ClientSessionServer *server, if (destroyNode == NULL) { continue; } + /* + * When the channel type is UDP and the business type is file, trigger DFILE_ON_CLEAR_POLICY_FILE_LIST event + * before cleaning up sessionNode. + */ + if (sessionNode->channelType == CHANNEL_TYPE_UDP && sessionNode->businessType == BUSINESS_TYPE_FILE) { + ClientEmitFileEvent(sessionNode->channelId); + } DestroySessionId(); ListDelete(&sessionNode->node); ListAdd(destroyList, &(destroyNode->node)); diff --git a/sdk/transmission/trans_channel/udp/common/include/client_trans_udp_manager.h b/sdk/transmission/trans_channel/udp/common/include/client_trans_udp_manager.h index 6f9b12c2ce..d9cb18ee0e 100644 --- a/sdk/transmission/trans_channel/udp/common/include/client_trans_udp_manager.h +++ b/sdk/transmission/trans_channel/udp/common/include/client_trans_udp_manager.h @@ -58,6 +58,7 @@ typedef struct { bool isEnable; sessionNeed info; int32_t routeType; + int32_t sessionId; } UdpChannel; int32_t ClientTransUdpMgrInit(IClientSessionCallBack *callback); @@ -84,6 +85,8 @@ void TransUdpDeleteFileListener(const char *sessionName); int32_t TransUdpOnCloseAckReceived(int32_t channelId); int32_t ClientEmitFileEvent(int32_t channelId); + +int32_t TransSetUdpChanelSessionId(int32_t channelId, int32_t sessionId); #ifdef __cplusplus } #endif diff --git a/sdk/transmission/trans_channel/udp/common/src/client_trans_udp_manager.c b/sdk/transmission/trans_channel/udp/common/src/client_trans_udp_manager.c index d2c6eb887a..c0fd7ab7b8 100644 --- a/sdk/transmission/trans_channel/udp/common/src/client_trans_udp_manager.c +++ b/sdk/transmission/trans_channel/udp/common/src/client_trans_udp_manager.c @@ -638,4 +638,29 @@ int32_t ClientEmitFileEvent(int32_t channelId) } } return ret; +} + +int32_t TransSetUdpChanelSessionId(int32_t channelId, int32_t sessionId) +{ + if (g_udpChannelMgr == NULL) { + TRANS_LOGE(TRANS_SDK, "udp channel manager hasn't init."); + return SOFTBUS_NO_INIT; + } + + if (SoftBusMutexLock(&(g_udpChannelMgr->lock)) != SOFTBUS_OK) { + TRANS_LOGE(TRANS_SDK, "lock failed"); + return SOFTBUS_LOCK_ERR; + } + + UdpChannel *channelNode = NULL; + LIST_FOR_EACH_ENTRY(channelNode, &(g_udpChannelMgr->list), UdpChannel, node) { + if (channelNode->channelId == channelId) { + channelNode->sessionId = sessionId; + (void)SoftBusMutexUnlock(&(g_udpChannelMgr->lock)); + return SOFTBUS_OK; + } + } + (void)SoftBusMutexUnlock(&(g_udpChannelMgr->lock)); + TRANS_LOGE(TRANS_SDK, "udp channel not found, channelId=%{public}d.", channelId); + return SOFTBUS_TRANS_UDP_CHANNEL_NOT_FOUND; } \ No newline at end of file diff --git a/sdk/transmission/trans_channel/udp/file/src/client_trans_file.c b/sdk/transmission/trans_channel/udp/file/src/client_trans_file.c index 7c7eaaf8c1..0aca8af86a 100644 --- a/sdk/transmission/trans_channel/udp/file/src/client_trans_file.c +++ b/sdk/transmission/trans_channel/udp/file/src/client_trans_file.c @@ -27,7 +27,6 @@ #include "trans_log.h" #define DEFAULT_KEY_LENGTH 32 -#define MAX_FILE_NUM 500 static const UdpChannelMgrCb *g_udpChannelMgrCb = NULL; @@ -65,10 +64,7 @@ static void FillFileStatusList(const DFileMsg *msgData, FileEvent *event) { int32_t fileNum = msgData->clearPolicyFileList.fileNum; if (fileNum <= 0) { - return; - } - if (fileNum > MAX_FILE_NUM) { - TRANS_LOGE(TRANS_SDK, "invalid fileNum."); + TRANS_LOGI(TRANS_SDK, "invalid fileNum."); return; } @@ -199,12 +195,39 @@ static void NotifySocketSendResult( FreeFileStatusList(&event); } -static void FileSendListener(int32_t dfileId, DFileMsgType msgType, const DFileMsg *msgData) +static bool IsParmasValid(DFileMsgType msgType, const DFileMsg *msgData) { - TRANS_LOGI(TRANS_FILE, "send dfileId=%{public}d type=%{public}d", dfileId, msgType); if (msgData == NULL || msgType == DFILE_ON_BIND || msgType == DFILE_ON_SESSION_IN_PROGRESS || msgType == DFILE_ON_SESSION_TRANSFER_RATE) { TRANS_LOGE(TRANS_SDK, "param invalid"); + return false; + } + return true; +} + +static void FileSendErrorEvent(UdpChannel *udpChannel, FileListener *fileListener, const DFileMsg *msgData, + DFileMsgType msgType, int32_t sessionId) +{ + if (fileListener->socketSendCallback != NULL) { + FileEvent event; + (void)memset_s(&event, sizeof(FileEvent), 0, sizeof(FileEvent)); + event.type = FILE_EVENT_SEND_ERROR; + FillFileStatusList(msgData, &event); + FillFileEventErrorCode(msgData, &event); + fileListener->socketSendCallback(sessionId, &event); + FreeFileStatusList(&event); + } else if (fileListener->sendListener.OnFileTransError != NULL) { + fileListener->sendListener.OnFileTransError(sessionId); + } + TRANS_LOGI(TRANS_SDK, "OnFile error. msgType=%{public}d", msgType); + TransOnUdpChannelClosed(udpChannel->channelId, SHUTDOWN_REASON_SEND_FILE_ERR); + return; +} + +static void FileSendListener(int32_t dfileId, DFileMsgType msgType, const DFileMsg *msgData) +{ + TRANS_LOGI(TRANS_FILE, "send dfileId=%{public}d type=%{public}d", dfileId, msgType); + if (!IsParmasValid(msgType, msgData)) { return; } UdpChannel udpChannel; @@ -224,6 +247,13 @@ static void FileSendListener(int32_t dfileId, DFileMsgType msgType, const DFileM if (TransGetFileListener(udpChannel.info.mySessionName, &fileListener) != SOFTBUS_OK) { return; } + if (msgType == DFILE_ON_CLEAR_POLICY_FILE_LIST) { + if (fileListener.socketSendCallback != NULL) { + NotifySocketSendResult(udpChannel.sessionId, msgType, msgData, &fileListener); + } + TRANS_LOGI(TRANS_SDK, "notify DFILE_ON_CLEAR_POLICY_FILE_LIST success."); + return; + } int32_t sessionId = -1; if (g_udpChannelMgrCb->OnFileGetSessionId(udpChannel.channelId, &sessionId) != SOFTBUS_OK) { @@ -232,19 +262,7 @@ static void FileSendListener(int32_t dfileId, DFileMsgType msgType, const DFileM } if (msgType == DFILE_ON_CONNECT_FAIL || msgType == DFILE_ON_FATAL_ERROR) { - if (fileListener.socketSendCallback != NULL) { - FileEvent event; - (void)memset_s(&event, sizeof(FileEvent), 0, sizeof(FileEvent)); - event.type = FILE_EVENT_SEND_ERROR; - FillFileStatusList(msgData, &event); - FillFileEventErrorCode(msgData, &event); - fileListener.socketSendCallback(sessionId, &event); - FreeFileStatusList(&event); - } else if (fileListener.sendListener.OnFileTransError != NULL) { - fileListener.sendListener.OnFileTransError(sessionId); - } - TRANS_LOGI(TRANS_SDK, "OnFile error. msgType=%{public}d", msgType); - TransOnUdpChannelClosed(udpChannel.channelId, SHUTDOWN_REASON_SEND_FILE_ERR); + FileSendErrorEvent(&udpChannel, &fileListener, msgData, msgType, sessionId); return; } (void)g_udpChannelMgrCb->OnIdleTimeoutReset(sessionId); @@ -333,12 +351,28 @@ static void NotifySocketRecvResult( FreeFileStatusList(&event); } +static void FileRecvErrorEvent(UdpChannel *udpChannel, FileListener *fileListener, const DFileMsg *msgData, + DFileMsgType msgType, int32_t sessionId) +{ + if (fileListener->socketRecvCallback != NULL) { + FileEvent event; + (void)memset_s(&event, sizeof(FileEvent), 0, sizeof(FileEvent)); + event.type = FILE_EVENT_RECV_ERROR; + FillFileStatusList(msgData, &event); + FillFileEventErrorCode(msgData, &event); + fileListener->socketRecvCallback(sessionId, &event); + FreeFileStatusList(&event); + } else if (fileListener->recvListener.OnFileTransError != NULL) { + fileListener->recvListener.OnFileTransError(sessionId); + } + TransOnUdpChannelClosed(udpChannel->channelId, SHUTDOWN_REASON_RECV_FILE_ERR); + return; +} + static void FileReceiveListener(int32_t dfileId, DFileMsgType msgType, const DFileMsg *msgData) { TRANS_LOGI(TRANS_FILE, "recv dfileId=%{public}d, type=%{public}d", dfileId, msgType); - if (msgData == NULL || msgType == DFILE_ON_BIND || msgType == DFILE_ON_SESSION_IN_PROGRESS || - msgType == DFILE_ON_SESSION_TRANSFER_RATE) { - TRANS_LOGE(TRANS_SDK, "param invalid"); + if (!IsParmasValid(msgType, msgData)) { return; } UdpChannel udpChannel; @@ -354,24 +388,20 @@ static void FileReceiveListener(int32_t dfileId, DFileMsgType msgType, const DFi TRANS_LOGE(TRANS_SDK, "get listener failed"); return; } + if (msgType == DFILE_ON_CLEAR_POLICY_FILE_LIST) { + if (fileListener.socketRecvCallback != NULL) { + NotifySocketRecvResult(udpChannel.sessionId, msgType, msgData, &fileListener); + } + TRANS_LOGI(TRANS_SDK, "notify DFILE_ON_CLEAR_POLICY_FILE_LIST success."); + return; + } int32_t sessionId = -1; if (g_udpChannelMgrCb->OnFileGetSessionId(udpChannel.channelId, &sessionId) != SOFTBUS_OK) { TRANS_LOGE(TRANS_SDK, "get sessionId failed"); return; } if (msgType == DFILE_ON_CONNECT_FAIL || msgType == DFILE_ON_FATAL_ERROR) { - if (fileListener.socketRecvCallback != NULL) { - FileEvent event; - (void)memset_s(&event, sizeof(FileEvent), 0, sizeof(FileEvent)); - event.type = FILE_EVENT_RECV_ERROR; - FillFileStatusList(msgData, &event); - FillFileEventErrorCode(msgData, &event); - fileListener.socketRecvCallback(sessionId, &event); - FreeFileStatusList(&event); - } else if (fileListener.recvListener.OnFileTransError != NULL) { - fileListener.recvListener.OnFileTransError(sessionId); - } - TransOnUdpChannelClosed(udpChannel.channelId, SHUTDOWN_REASON_RECV_FILE_ERR); + FileRecvErrorEvent(&udpChannel, &fileListener, msgData, msgType, sessionId); return; } (void)g_udpChannelMgrCb->OnIdleTimeoutReset(sessionId); diff --git a/tests/sdk/transmission/session/BUILD.gn b/tests/sdk/transmission/session/BUILD.gn index 319ea57a23..a85c367d60 100644 --- a/tests/sdk/transmission/session/BUILD.gn +++ b/tests/sdk/transmission/session/BUILD.gn @@ -117,6 +117,7 @@ ohos_unittest("TransClientSessionCallbackTest") { "$dsoftbus_root_path/core/bus_center/lnn/net_ledger/common/include", "$dsoftbus_root_path/core/discovery/manager/include", "$dsoftbus_root_path/core/discovery/interface", + "$dsoftbus_root_path/sdk/transmission/trans_channel/udp/common/include", ] deps = [ diff --git a/tests/sdk/transmission/trans_channel/udp/file/trans_sdk_file_test.cpp b/tests/sdk/transmission/trans_channel/udp/file/trans_sdk_file_test.cpp index 246e801ae7..3c4f75e492 100644 --- a/tests/sdk/transmission/trans_channel/udp/file/trans_sdk_file_test.cpp +++ b/tests/sdk/transmission/trans_channel/udp/file/trans_sdk_file_test.cpp @@ -888,16 +888,6 @@ HWTEST_F(TransSdkFileTest, FillFileStatusListTest002, TestSize.Level0) ASSERT_EQ(0, event.statusList.notCompletedList.fileCnt); ASSERT_EQ(nullptr, event.statusList.notStartedList.files); ASSERT_EQ(0, event.statusList.notStartedList.fileCnt); - - msgData.clearPolicyFileList.fileNum = MAX_FILE_NUM + 1; - FillFileStatusList(&msgData, &event); - // Check status list content - ASSERT_EQ(nullptr, event.statusList.completedList.files); - ASSERT_EQ(0, event.statusList.completedList.fileCnt); - ASSERT_EQ(nullptr, event.statusList.notCompletedList.files); - ASSERT_EQ(0, event.statusList.notCompletedList.fileCnt); - ASSERT_EQ(nullptr, event.statusList.notStartedList.files); - ASSERT_EQ(0, event.statusList.notStartedList.fileCnt); } /**