From 1b6339a7ed63732161d3bbfa44cd585d3aed8e07 Mon Sep 17 00:00:00 2001
From: Simon Morlat <simon.morlat@linphone.org>
Date: Fri, 2 Jun 2023 10:06:45 +0200
Subject: [PATCH] Test server moved to fs-test-7. Fix null pointer crash and
double free in ServerGroupChatRoom
---
src/c-wrapper/internal/c-tools.h | 9 +++++++-
src/chat/chat-room/server-group-chat-room-p.h | 6 ++---
src/chat/chat-room/server-group-chat-room.cpp | 23 +++++++++----------
tester/tester.c | 3 ++-
4 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/c-wrapper/internal/c-tools.h b/src/c-wrapper/internal/c-tools.h
index 9310128d62..46347fb282 100644
--- a/src/c-wrapper/internal/c-tools.h
+++ b/src/c-wrapper/internal/c-tools.h
@@ -231,7 +231,14 @@ public:
wrappedObject->weakCppPtr.~weak_ptr();
}
- template <typename CType,
+ template<typename CppType>
+ static bool isOwnedByC(void *cObject){
+ WrappedBaseObject<CppType> *base = static_cast<WrappedBaseObject<CppType>*>(cObject);
+ return base->owner == WrappedObjectOwner::External;
+ }
+
+ template<
+ typename CType,
typename CppType = typename CTypeMetaInfo<CType>::cppType,
typename = typename std::enable_if<IsDefinedClonableCppObject<CppType>::value, CppType>::type>
static void uninitClonableCppObject(CType *cObject) {
diff --git a/src/chat/chat-room/server-group-chat-room-p.h b/src/chat/chat-room/server-group-chat-room-p.h
index 0a46c9ee86..f8767c56ce 100644
--- a/src/chat/chat-room/server-group-chat-room-p.h
+++ b/src/chat/chat-room/server-group-chat-room-p.h
@@ -193,12 +193,10 @@ private:
std::map<std::string, RegistrationSubscriptionContext>
registrationSubscriptions; /*map of registrationSubscriptions for each participant*/
int unnotifiedRegistrationSubscriptions = 0; /*count of not-yet notified registration subscriptions*/
- std::shared_ptr<ParticipantDevice>
- mInitiatorDevice; /*pointer to the ParticipantDevice that is creating the chat room*/
- bool joiningPendingAfterCreation = false;
- bool needsUnref = false;
+ std::shared_ptr<ParticipantDevice> mInitiatorDevice; /*pointer to the ParticipantDevice that is creating the chat room*/
std::unordered_map<std::string, std::queue<std::shared_ptr<Message>>> queuedMessages;
Utils::Version protocolVersion;
+ bool joiningPendingAfterCreation = false;
L_DECLARE_PUBLIC(ServerGroupChatRoom);
};
diff --git a/src/chat/chat-room/server-group-chat-room.cpp b/src/chat/chat-room/server-group-chat-room.cpp
index caec26bec5..0f9546a883 100644
--- a/src/chat/chat-room/server-group-chat-room.cpp
+++ b/src/chat/chat-room/server-group-chat-room.cpp
@@ -223,12 +223,13 @@ void ServerGroupChatRoomPrivate::requestDeletion() {
* to make the self-destruction outside of the call stack that leaded to it.
* TODO: remove this after switching chatrooms to HybridObject.
*/
- if (needsUnref) {
- LinphoneChatRoom *cChatRoom = L_GET_C_BACK_PTR(chatRoom);
- /* If the chatroom was created as "external" mode, explicitely unref the C object to destroy it.*/
- if (cChatRoom) {
- q->getCore()->doLater([cChatRoom]() { linphone_chat_room_unref(cChatRoom); });
- }
+
+ LinphoneChatRoom *cChatRoom = L_GET_C_BACK_PTR(chatRoom);
+ /* If the chatroom was created as "external" mode, explicitely unref the C object to destroy it.*/
+ if (cChatRoom && Wrapper::isOwnedByC<ServerGroupChatRoom>(cChatRoom)){
+ q->getCore()->doLater([cChatRoom](){
+ linphone_chat_room_unref(cChatRoom);
+ });
}
}
@@ -1270,6 +1271,10 @@ void ServerGroupChatRoomPrivate::onBye(const shared_ptr<ParticipantDevice> &part
bool ServerGroupChatRoomPrivate::dispatchMessagesAfterFullState(const shared_ptr<CallSession> &session) const {
L_Q();
auto device = q->findCachedParticipantDevice(session);
+ if (!device) {
+ lWarning() << q << " dispatchMessagesAfterFullState on unknown device.";
+ return false; // Assume it is a recent device.
+ }
return dispatchMessagesAfterFullState(device);
}
@@ -1425,12 +1430,6 @@ ServerGroupChatRoom::ServerGroupChatRoom(const shared_ptr<Core> &core, SalCallOp
session->configure(LinphoneCallIncoming, nullptr, op, from, to);
d->protocolVersion = CorePrivate::groupChatProtocolVersion;
- /*
- * HACK: see comment in ServerGroupChatRoomPrivate::requestDeletion() for details.
- * When this constructor is used, a reference to the C object is given to the core.
- * Thus, remember that it will have to be released.
- */
- d->needsUnref = true;
}
ServerGroupChatRoom::ServerGroupChatRoom(const shared_ptr<Core> &core,
diff --git a/tester/tester.c b/tester/tester.c
index 58aee4bdb6..ca36c07dd6 100644
--- a/tester/tester.c
+++ b/tester/tester.c
@@ -86,7 +86,8 @@ const MSAudioDiffParams audio_cmp_params = {10, 200};
/* Default test server infrastructure. You may change to sandbox infrastructure to test changes to the infrastructure
* first. */
-const char *flexisip_tester_dns_server = "fs-test-3.linphone.org";
+const char *flexisip_tester_dns_server = "fs-test-7.linphone.org";
+
// const char* flexisip_tester_dns_server = "fs-test-sandbox.linphone.org";
bctbx_list_t *flexisip_tester_dns_ip_addresses = NULL;
--
GitLab