From 880eb5815547e9144a1f73d72ddaf2ada8b7217d Mon Sep 17 00:00:00 2001 From: csoler Date: Sun, 23 Jul 2017 22:56:27 +0200 Subject: [PATCH 01/14] suppressed memory leak in serialisation test code --- tests/unittests/libretroshare/serialiser/support.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unittests/libretroshare/serialiser/support.h b/tests/unittests/libretroshare/serialiser/support.h index f681d90ad..0b042701c 100644 --- a/tests/unittests/libretroshare/serialiser/support.h +++ b/tests/unittests/libretroshare/serialiser/support.h @@ -173,10 +173,9 @@ template int test_RsItem() EXPECT_TRUE(done2) ; EXPECT_TRUE(sersize2 == sersize); -// displayRawPacket(std::cerr, (void *) buffer, 16 * 8 + sersize2); - + delete output ; delete[] buffer ; - //delete rsfis; + delete rsfis; return 1; } From 764fadf0ee2c02b9f8ee7b959f4f088ebc1a9072 Mon Sep 17 00:00:00 2001 From: csoler Date: Sun, 23 Jul 2017 22:58:01 +0200 Subject: [PATCH 02/14] suppressed memory leak in serialisation test code --- tests/unittests/libretroshare/serialiser/support.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unittests/libretroshare/serialiser/support.h b/tests/unittests/libretroshare/serialiser/support.h index 0b042701c..d9d5ca48c 100644 --- a/tests/unittests/libretroshare/serialiser/support.h +++ b/tests/unittests/libretroshare/serialiser/support.h @@ -239,6 +239,8 @@ template int test_RsItem(uint16_t servtype) // displayRawPacket(std::cerr, (void *) buffer, 16 * 8 + sersize2); delete[] buffer ; + delete output ; + delete rsfis ; return 1; } From df691bd2d75bbfcc7ba51c4722c7a0e4e1b276a9 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 24 Jul 2017 12:16:06 +0200 Subject: [PATCH 03/14] improved tests to avoid memory leak --- .../libretroshare/gxs/common/data_support.cc | 37 +++++---- .../libretroshare/gxs/common/data_support.h | 14 ++-- .../gxs/data_service/rsdataservice_test.cc | 4 +- .../serialiser/rsgxsiditem_test.cc | 6 +- .../serialiser/rsgxsupdateitem_test.cc | 22 ++--- .../serialiser/rsmsgitem_test.cc | 83 +++++++------------ .../serialiser/rsnxsitems_test.cc | 14 ++-- .../serialiser/rsstatusitem_test.cc | 5 +- .../libretroshare/serialiser/rstlvutil.cc | 2 + .../serialiser/rsturtleitem_test.cc | 11 +-- .../libretroshare/serialiser/support.h | 23 ++--- .../libretroshare/serialiser/tlvstack_test.cc | 40 ++++----- 12 files changed, 120 insertions(+), 141 deletions(-) diff --git a/tests/unittests/libretroshare/gxs/common/data_support.cc b/tests/unittests/libretroshare/gxs/common/data_support.cc index 1b64e8e8c..ce716c62a 100644 --- a/tests/unittests/libretroshare/gxs/common/data_support.cc +++ b/tests/unittests/libretroshare/gxs/common/data_support.cc @@ -151,7 +151,7 @@ void init_item(RsGxsMsgMetaData* metaMsg) -RsSerialType* init_item(RsNxsGrp& nxg) +void init_item(RsNxsGrp& nxg,RsSerialType **ser) { nxg.clear(); @@ -160,11 +160,12 @@ RsSerialType* init_item(RsNxsGrp& nxg) init_item(nxg.grp); init_item(nxg.meta); - return new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + if(ser) + *ser = new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsNxsMsg& nxm) +void init_item(RsNxsMsg& nxm,RsSerialType **ser) { nxm.clear(); @@ -174,20 +175,22 @@ RsSerialType* init_item(RsNxsMsg& nxm) init_item(nxm.meta); nxm.transactionNumber = rand()%23; - return new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + if(ser) + *ser = new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsNxsSyncGrpReqItem& rsg) +void init_item(RsNxsSyncGrpReqItem& rsg,RsSerialType **ser) { rsg.clear(); rsg.flag = RsNxsSyncGrpItem::FLAG_USE_SYNC_HASH; rsg.createdSince = rand()%2423; randString(3124,rsg.syncHash); - return new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + if(ser) + *ser = new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsNxsSyncMsgReqItem& rsgm) +void init_item(RsNxsSyncMsgReqItem& rsgm,RsSerialType **ser) { rsgm.clear(); @@ -197,10 +200,11 @@ RsSerialType* init_item(RsNxsSyncMsgReqItem& rsgm) init_random(rsgm.grpId) ; randString(SHORT_STR, rsgm.syncHash); - return new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + if(ser) + *ser = new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsNxsSyncGrpItem& rsgl) +void init_item(RsNxsSyncGrpItem& rsgl,RsSerialType **ser) { rsgl.clear(); @@ -209,10 +213,11 @@ RsSerialType* init_item(RsNxsSyncGrpItem& rsgl) rsgl.publishTs = rand()%23; init_random(rsgl.grpId) ; - return new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + if(ser) + *ser = new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsNxsSyncMsgItem& rsgml) +void init_item(RsNxsSyncMsgItem& rsgml,RsSerialType **ser) { rsgml.clear(); @@ -221,11 +226,12 @@ RsSerialType* init_item(RsNxsSyncMsgItem& rsgml) init_random(rsgml.grpId) ; init_random(rsgml.msgId) ; - return new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + if(ser) + *ser = new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsNxsTransacItem &rstx){ - +void init_item(RsNxsTransacItem &rstx,RsSerialType **ser) +{ rstx.clear(); rstx.timestamp = rand()%14141; @@ -233,7 +239,8 @@ RsSerialType* init_item(RsNxsTransacItem &rstx){ rstx.nItems = rand()%33132; rstx.transactionNumber = rand()%242112; - return new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + if(ser) + *ser = new RsNxsSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } diff --git a/tests/unittests/libretroshare/gxs/common/data_support.h b/tests/unittests/libretroshare/gxs/common/data_support.h index 0bb026820..674703488 100644 --- a/tests/unittests/libretroshare/gxs/common/data_support.h +++ b/tests/unittests/libretroshare/gxs/common/data_support.h @@ -21,13 +21,13 @@ void init_item(RsGxsGrpMetaData* metaGrp); void init_item(RsGxsMsgMetaData* metaMsg); -RsSerialType* init_item(RsNxsGrp& nxg); -RsSerialType* init_item(RsNxsMsg& nxm); -RsSerialType* init_item(RsNxsSyncGrpReqItem &rsg); -RsSerialType* init_item(RsNxsSyncMsgReqItem &rsgm); -RsSerialType* init_item(RsNxsSyncGrpItem& rsgl); -RsSerialType* init_item(RsNxsSyncMsgItem& rsgml); -RsSerialType* init_item(RsNxsTransacItem& rstx); +void init_item(RsNxsGrp& nxg ,RsSerialType ** = NULL); +void init_item(RsNxsMsg& nxm ,RsSerialType ** = NULL); +void init_item(RsNxsSyncGrpReqItem &rsg ,RsSerialType ** = NULL); +void init_item(RsNxsSyncMsgReqItem &rsgm,RsSerialType ** = NULL); +void init_item(RsNxsSyncGrpItem& rsgl ,RsSerialType ** = NULL); +void init_item(RsNxsSyncMsgItem& rsgml ,RsSerialType ** = NULL); +void init_item(RsNxsTransacItem& rstx ,RsSerialType ** = NULL); template void copy_all_but(T& ex, const std::list& s, std::list& d) diff --git a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc index 9720c1294..5874c303f 100644 --- a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc +++ b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc @@ -5,6 +5,7 @@ #include "libretroshare/gxs/common/data_support.h" #include "rsdataservice_test.h" #include "gxs/rsgds.h" +#include "gxs/rsgxsutil.h" #include "gxs/rsdataservice.h" #define DATA_BASE_NAME "msg_grp_Store" @@ -221,7 +222,8 @@ void test_messageStoresAndRetrieve() // first selective retrieval GxsMsgResult msgResult; - GxsMsgMetaResult msgMetaResult; + + RsGxsMetaDataTemporaryMapVector msgMetaResult ; dStore->retrieveNxsMsgs(req, msgResult, false); dStore->retrieveGxsMsgMetaData(req, msgMetaResult); diff --git a/tests/unittests/libretroshare/serialiser/rsgxsiditem_test.cc b/tests/unittests/libretroshare/serialiser/rsgxsiditem_test.cc index e1488441f..c1aaa0fc0 100644 --- a/tests/unittests/libretroshare/serialiser/rsgxsiditem_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsgxsiditem_test.cc @@ -38,11 +38,9 @@ bool operator==(const RsGxsIdGroupItem& it1,const RsGxsIdGroupItem& it2) return true ; } -RsSerialType* init_item(RsGxsIdGroupItem& item) +void init_item(RsGxsIdGroupItem& item) { item.mPgpIdSign = "hello"; - - return new RsGxsIdSerialiser(); } @@ -50,7 +48,7 @@ TEST(libretroshare_serialiser, RsGxsIdItem) { for(uint32_t i=0;i<20;++i) { - test_RsItem< RsGxsIdGroupItem >(); + test_RsItem< RsGxsIdGroupItem,RsGxsIdSerialiser >(); } } diff --git a/tests/unittests/libretroshare/serialiser/rsgxsupdateitem_test.cc b/tests/unittests/libretroshare/serialiser/rsgxsupdateitem_test.cc index ccaeead96..9ece9edce 100644 --- a/tests/unittests/libretroshare/serialiser/rsgxsupdateitem_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsgxsupdateitem_test.cc @@ -11,15 +11,14 @@ #include "rsitems/rsgxsupdateitems.h" #define RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM 0x0010 -RsSerialType* init_item(RsGxsGrpUpdateItem& i) +void init_item(RsGxsGrpUpdateItem& i) { i.clear(); i.grpUpdateTS = rand()%2424; i.peerID = RsPeerId::random(); - return new RsGxsUpdateSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsGxsMsgUpdateItem& i) +void init_item(RsGxsMsgUpdateItem& i) { i.clear(); i.peerID = RsPeerId::random(); @@ -33,24 +32,19 @@ RsSerialType* init_item(RsGxsMsgUpdateItem& i) info.time_stamp = rand()%45; i.msgUpdateInfos[RsGxsGroupId::random()] = info; } - - return new RsGxsUpdateSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsGxsServerGrpUpdateItem& i) +void init_item(RsGxsServerGrpUpdateItem& i) { i.clear(); i.grpUpdateTS = rand()%2424; - - return new RsGxsUpdateSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } -RsSerialType* init_item(RsGxsServerMsgUpdateItem& i) +void init_item(RsGxsServerMsgUpdateItem& i) { i.clear(); i.grpId = RsGxsGroupId::random(); i.msgUpdateTS = rand()%4252; - return new RsGxsUpdateSerialiser(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } bool operator ==(const RsGxsGrpUpdateItem& l, const RsGxsGrpUpdateItem& r) @@ -105,8 +99,8 @@ bool operator ==(const RsGxsServerMsgUpdateItem& l, TEST(libretroshare_serialiser, RsGxsGrpUpateItem) { - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } diff --git a/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc b/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc index 1d7d1258c..a2b1369ad 100644 --- a/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc @@ -33,19 +33,16 @@ #include "rstlvutil.h" -RsSerialType* init_item(RsChatMsgItem& cmi) +void init_item(RsChatMsgItem& cmi) { cmi.chatFlags = rand()%34; cmi.sendTime = rand()%422224; randString(LARGE_STR, cmi.message); - - return new RsChatSerialiser(); } -RsSerialType* init_item(RsChatLobbyListRequestItem& ) +void init_item(RsChatLobbyListRequestItem& ) { - return new RsChatSerialiser(); } -RsSerialType* init_item(RsChatLobbyListItem& cmi) +void init_item(RsChatLobbyListItem& cmi) { int n = rand()%20 ; @@ -59,41 +56,33 @@ RsSerialType* init_item(RsChatLobbyListItem& cmi) info.flags = ChatLobbyFlags(RSRandom::random_u32()%3) ; cmi.lobbies.push_back(info); } - - return new RsChatSerialiser(); } -RsSerialType* init_item(RsChatLobbyMsgItem& cmi) +void init_item(RsChatLobbyMsgItem& cmi) { - RsSerialType *serial = init_item( *dynamic_cast(&cmi)) ; + init_item( *dynamic_cast(&cmi)) ; cmi.msg_id = RSRandom::random_u64() ; cmi.lobby_id = RSRandom::random_u64() ; cmi.nick = "My nickname" ; cmi.parent_msg_id = RSRandom::random_u64() ; - - return serial ; } -RsSerialType *init_item(RsChatLobbyEventItem& cmi) +void init_item(RsChatLobbyEventItem& cmi) { cmi.lobby_id = RSRandom::random_u64() ; cmi.msg_id = RSRandom::random_u64() ; randString(20, cmi.nick); cmi.event_type = RSRandom::random_u32()%256 ; randString(20, cmi.string1); - - return new RsChatSerialiser(); } -RsSerialType* init_item(RsChatLobbyInviteItem& cmi) +void init_item(RsChatLobbyInviteItem& cmi) { cmi.lobby_id = RSRandom::random_u64() ; cmi.lobby_name = "Name of the lobby" ; cmi.lobby_topic = "Topic of the lobby" ; - - return new RsChatSerialiser(); } -RsSerialType* init_item(RsPrivateChatMsgConfigItem& pcmi) +void init_item(RsPrivateChatMsgConfigItem& pcmi) { pcmi.configPeerId = RsPeerId::random(); pcmi.chatFlags = rand()%34; @@ -101,21 +90,17 @@ RsSerialType* init_item(RsPrivateChatMsgConfigItem& pcmi) pcmi.sendTime = rand()%422224; randString(LARGE_STR, pcmi.message); pcmi.recvTime = rand()%344443; - - return new RsChatSerialiser(); } -RsSerialType* init_item(RsChatStatusItem& csi) +void init_item(RsChatStatusItem& csi) { randString(SHORT_STR, csi.status_string); csi.flags = rand()%232; - return new RsChatSerialiser(); - } -RsSerialType* init_item(RsChatAvatarItem& cai) +void init_item(RsChatAvatarItem& cai) { std::string image_data; randString(LARGE_STR, image_data); @@ -123,11 +108,9 @@ RsSerialType* init_item(RsChatAvatarItem& cai) memcpy(cai.image_data, image_data.c_str(), image_data.size()); cai.image_size = image_data.size(); - - return new RsChatSerialiser(); } -RsSerialType* init_item(RsMsgItem& mi) +void init_item(RsMsgItem& mi) { init_item(mi.attachment); init_item(mi.rspeerid_msgbcc); @@ -141,21 +124,17 @@ RsSerialType* init_item(RsMsgItem& mi) mi.recvTime = rand()%44252; mi.sendTime = mi.recvTime; mi.msgFlags = mi.recvTime; - - return new RsMsgSerialiser(RsServiceSerializer::SERIALIZATION_FLAG_NONE); } -RsSerialType* init_item(RsMsgTagType& mtt) +void init_item(RsMsgTagType& mtt) { mtt.rgb_color = rand()%5353; mtt.tagId = rand()%24242; randString(SHORT_STR, mtt.text); - - return new RsMsgSerialiser(RsServiceSerializer::SERIALIZATION_FLAG_NONE); } -RsSerialType* init_item(RsMsgTags& mt) +void init_item(RsMsgTags& mt) { mt.msgId = rand()%3334; @@ -163,24 +142,18 @@ RsSerialType* init_item(RsMsgTags& mt) for (i = 0; i < 10; i++) { mt.tagIds.push_back(rand()%21341); } - - return new RsMsgSerialiser(RsServiceSerializer::SERIALIZATION_FLAG_NONE); } -RsSerialType* init_item(RsMsgSrcId& ms) +void init_item(RsMsgSrcId& ms) { ms.msgId = rand()%434; ms.srcId = RsPeerId::random(); - - return new RsMsgSerialiser(RsServiceSerializer::SERIALIZATION_FLAG_NONE); } -RsSerialType* init_item(RsMsgParentId& ms) +void init_item(RsMsgParentId& ms) { ms.msgId = rand()%354; ms.msgParentId = rand()%476; - - return new RsMsgSerialiser(RsServiceSerializer::SERIALIZATION_FLAG_NONE); } bool operator ==(const struct VisibleChatLobbyInfo& l, const struct VisibleChatLobbyInfo& r) @@ -335,19 +308,19 @@ bool operator ==(const RsMsgParentId& msLeft, const RsMsgParentId& msRight) TEST(libretroshare_serialiser, RsMsgItem) { - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); } diff --git a/tests/unittests/libretroshare/serialiser/rsnxsitems_test.cc b/tests/unittests/libretroshare/serialiser/rsnxsitems_test.cc index 87bd33778..0ff8f7f44 100644 --- a/tests/unittests/libretroshare/serialiser/rsnxsitems_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsnxsitems_test.cc @@ -10,11 +10,11 @@ TEST(libretroshare_serialiser, RsNxsItem) { - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + test_RsItem(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); } diff --git a/tests/unittests/libretroshare/serialiser/rsstatusitem_test.cc b/tests/unittests/libretroshare/serialiser/rsstatusitem_test.cc index 1f7e53f14..48d037cca 100644 --- a/tests/unittests/libretroshare/serialiser/rsstatusitem_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsstatusitem_test.cc @@ -27,12 +27,11 @@ #include "support.h" #include "rsitems/rsstatusitems.h" -RsSerialType* init_item(RsStatusItem& rsi) +void init_item(RsStatusItem& rsi) { rsi.sendTime = rand()%5353; rsi.status = rand()%2032; - return new RsStatusSerialiser(); } bool operator ==(RsStatusItem& rsi1, RsStatusItem& rsi2) @@ -48,5 +47,5 @@ bool operator ==(RsStatusItem& rsi1, RsStatusItem& rsi2) TEST(libretroshare_serialiser, test_RsStatusItem) { - test_RsItem(); + test_RsItem(); } diff --git a/tests/unittests/libretroshare/serialiser/rstlvutil.cc b/tests/unittests/libretroshare/serialiser/rstlvutil.cc index 4ca2ab0e4..e468d49ad 100644 --- a/tests/unittests/libretroshare/serialiser/rstlvutil.cc +++ b/tests/unittests/libretroshare/serialiser/rstlvutil.cc @@ -172,6 +172,8 @@ int test_TlvSet(std::vector items, int maxsize) test_CreateTlvStack(std::cerr, items, data, &size); test_StepThroughTlvStack(std::cerr, data, size); + free(data) ; + return 1; } diff --git a/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc b/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc index 55a6add4b..3590d519a 100644 --- a/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc @@ -237,11 +237,12 @@ TEST(libretroshare_serialiser, RsTurtleItem) //test_RsItem(); //test_RsItem(); //test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); - test_RsItem(); + + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); + test_RsItem(); } } diff --git a/tests/unittests/libretroshare/serialiser/support.h b/tests/unittests/libretroshare/serialiser/support.h index d9d5ca48c..76ed3f359 100644 --- a/tests/unittests/libretroshare/serialiser/support.h +++ b/tests/unittests/libretroshare/serialiser/support.h @@ -100,15 +100,17 @@ bool operator==(const RsTlvKeySignatureSet& , const RsTlvKeySignatureSet& ); * @param T the item you want to test */ -template int test_RsItem() +template int test_RsItem() { /* make a serialisable RsTurtleItem */ RsSerialiser srl; /* initialise */ - T rsfi ; - RsSerialType *rsfis = init_item(rsfi) ; + ItemClass rsfi ; + RsSerialType *rsfis = new ItemSerialiser; + + init_item(rsfi); /* attempt to serialise it before we add it to the serialiser */ std::cerr << "### These errors are expected." << std::endl; @@ -149,7 +151,7 @@ template int test_RsItem() EXPECT_TRUE(sersize2 == sersize); - T *outfi = dynamic_cast(output); + ItemClass *outfi = dynamic_cast(output); EXPECT_TRUE(outfi != NULL); if (!outfi) @@ -173,22 +175,24 @@ template int test_RsItem() EXPECT_TRUE(done2) ; EXPECT_TRUE(sersize2 == sersize); + std::cerr << "Deleting output" < int test_RsItem(uint16_t servtype) +template int test_RsItem(uint16_t servtype) { /* make a serialisable RsTurtleItem */ RsSerialiser srl; /* initialise */ - T rsfi(servtype) ; - RsSerialType *rsfis = init_item(rsfi) ; // deleted on destruction of srl + ItemClass rsfi(servtype) ; + RsSerialType *rsfis = new ItemSerialiser(servtype) ; + + init_item(rsfi) ; // deleted on destruction of srl /* attempt to serialise it before we add it to the serialiser */ std::cerr << "### These errors are expected." << std::endl; @@ -223,7 +227,7 @@ template int test_RsItem(uint16_t servtype) EXPECT_TRUE(output != NULL); EXPECT_TRUE(sersize2 == sersize); - T *outfi = dynamic_cast(output); + ItemClass *outfi = dynamic_cast(output); EXPECT_TRUE(outfi != NULL); @@ -240,7 +244,6 @@ template int test_RsItem(uint16_t servtype) delete[] buffer ; delete output ; - delete rsfis ; return 1; } diff --git a/tests/unittests/libretroshare/serialiser/tlvstack_test.cc b/tests/unittests/libretroshare/serialiser/tlvstack_test.cc index d4d46ed0e..6bcfaa513 100644 --- a/tests/unittests/libretroshare/serialiser/tlvstack_test.cc +++ b/tests/unittests/libretroshare/serialiser/tlvstack_test.cc @@ -45,8 +45,8 @@ TEST(libretroshare_serialiser, test_RsTlvStack) /* now create a set of TLV items for the random generator */ - RsTlvBinaryData *bd1 = new RsTlvBinaryData(123); - RsTlvBinaryData *bd2 = new RsTlvBinaryData(125); + RsTlvBinaryData bd1(123); + RsTlvBinaryData bd2(125); char data[BIN_LEN] = {0}; int i; @@ -55,31 +55,31 @@ TEST(libretroshare_serialiser, test_RsTlvStack) data[i] = i%13; } - bd1->setBinData(data, 5); - bd2->setBinData(data, 21); + bd1.setBinData(data, 5); + bd2.setBinData(data, 21); - RsTlvFileItem *fi1 = new RsTlvFileItem(); - RsTlvFileItem *fi2 = new RsTlvFileItem(); + RsTlvFileItem fi1; + RsTlvFileItem fi2; /* initialise */ - fi1->filesize = 101010; - fi1->hash = RsFileHash("123456789ABCDEF67890123456789ABCDEF67890");//SHA1_SIZE*2 = 40 - fi1->name = "TestFile.txt"; - fi1->pop = 12; - fi1->age = 456; + fi1.filesize = 101010; + fi1.hash = RsFileHash("123456789ABCDEF67890123456789ABCDEF67890");//SHA1_SIZE*2 = 40 + fi1.name = "TestFile.txt"; + fi1.pop = 12; + fi1.age = 456; - fi2->filesize = 101010; - fi2->hash = RsFileHash("123456789ABCDEF67890123456789ABCDEF67890");//SHA1_SIZE*2 = 40 - fi2->name = "TestFile.txt"; - fi2->pop = 0; - fi2->age = 0;; + fi2.filesize = 101010; + fi2.hash = RsFileHash("123456789ABCDEF67890123456789ABCDEF67890");//SHA1_SIZE*2 = 40 + fi2.name = "TestFile.txt"; + fi2.pop = 0; + fi2.age = 0;; std::vector items; items.resize(4); - items[0] = bd1; - items[1] = bd2; - items[2] = fi1; - items[3] = fi2; + items[0] = &bd1; + items[1] = &bd2; + items[2] = &fi1; + items[3] = &fi2; test_TlvSet(items, 1024); } From bcaafc241df8e39f4b9f093872f1ee3b41eb8815 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 24 Jul 2017 14:29:30 +0200 Subject: [PATCH 04/14] improved template class naming for temporary Gxs grp data structures --- libretroshare/src/gxs/rsgxsnetservice.cc | 29 +++++++++---------- libretroshare/src/gxs/rsgxsutil.h | 23 ++++++++++----- libretroshare/src/rsitems/rsmsgitems.h | 2 +- libretroshare/src/services/p3gxscircles.cc | 2 +- .../gxs/data_service/rsdataservice_test.cc | 26 +++++++---------- 5 files changed, 42 insertions(+), 40 deletions(-) diff --git a/libretroshare/src/gxs/rsgxsnetservice.cc b/libretroshare/src/gxs/rsgxsnetservice.cc index a25d18ca4..88c7701d2 100644 --- a/libretroshare/src/gxs/rsgxsnetservice.cc +++ b/libretroshare/src/gxs/rsgxsnetservice.cc @@ -551,14 +551,13 @@ void RsGxsNetService::syncWithPeers() #ifndef GXS_DISABLE_SYNC_MSGS - typedef RsGxsMetaDataTemporaryMap GrpMetaMap; - GrpMetaMap grpMeta; + RsGxsGrpMetaTemporaryMap grpMeta; mDataStore->retrieveGxsGrpMetaData(grpMeta); - GrpMetaMap toRequest; + RsGxsGrpMetaTemporaryMap toRequest; - for(GrpMetaMap::iterator mit = grpMeta.begin(); mit != grpMeta.end(); ++mit) + for(RsGxsGrpMetaTemporaryMap::iterator mit = grpMeta.begin(); mit != grpMeta.end(); ++mit) { RsGxsGrpMetaData* meta = mit->second; @@ -596,7 +595,7 @@ void RsGxsNetService::syncWithPeers() GXSNETDEBUG_P_(peerId) << " syncing messages with peer " << peerId << std::endl; #endif - GrpMetaMap::const_iterator mmit = toRequest.begin(); + RsGxsGrpMetaTemporaryMap::const_iterator mmit = toRequest.begin(); for(; mmit != toRequest.end(); ++mmit) { const RsGxsGrpMetaData* meta = mmit->second; @@ -678,7 +677,7 @@ void RsGxsNetService::syncGrpStatistics() #ifdef NXS_NET_DEBUG_6 GXSNETDEBUG___<< "Sync-ing group statistics." << std::endl; #endif - RsGxsMetaDataTemporaryMap grpMeta; + RsGxsGrpMetaTemporaryMap grpMeta; mDataStore->retrieveGxsGrpMetaData(grpMeta); @@ -751,7 +750,7 @@ void RsGxsNetService::handleRecvSyncGrpStatistics(RsNxsSyncGrpStatsItem *grs) #ifdef NXS_NET_DEBUG_6 GXSNETDEBUG_PG(grs->PeerId(),grs->grpId) << "Received Grp update stats Request for group " << grs->grpId << " from friend " << grs->PeerId() << std::endl; #endif - RsGxsMetaDataTemporaryMap grpMetas; + RsGxsGrpMetaTemporaryMap grpMetas; grpMetas[grs->grpId] = NULL; mDataStore->retrieveGxsGrpMetaData(grpMetas); @@ -1905,7 +1904,7 @@ void RsGxsNetService::updateClientSyncTS() void RsGxsNetService::updateServerSyncTS() { - RsGxsMetaDataTemporaryMap gxsMap; + RsGxsGrpMetaTemporaryMap gxsMap; #ifdef NXS_NET_DEBUG_0 GXSNETDEBUG___<< "updateServerSyncTS(): updating last modification time stamp of local data." << std::endl; @@ -2719,7 +2718,7 @@ void RsGxsNetService::locked_genReqMsgTransaction(NxsTransaction* tr) GXSNETDEBUG_PG(item->PeerId(),grpId) << " grpId = " << grpId << std::endl; GXSNETDEBUG_PG(item->PeerId(),grpId) << " retrieving grp mesta data..." << std::endl; #endif - RsGxsMetaDataTemporaryMap grpMetaMap; + RsGxsGrpMetaTemporaryMap grpMetaMap; grpMetaMap[grpId] = NULL; mDataStore->retrieveGxsGrpMetaData(grpMetaMap); @@ -2977,7 +2976,7 @@ void RsGxsNetService::locked_genReqGrpTransaction(NxsTransaction* tr) #endif std::list grpItemL; - RsGxsMetaDataTemporaryMap grpMetaMap; + RsGxsGrpMetaTemporaryMap grpMetaMap; for(std::list::iterator lit = tr->mItems.begin(); lit != tr->mItems.end(); ++lit) { @@ -3086,7 +3085,7 @@ void RsGxsNetService::locked_genSendGrpsTransaction(NxsTransaction* tr) std::list::iterator lit = tr->mItems.begin(); - RsGxsMetaDataTemporaryMap grps ; + t_RsGxsGenericDataTemporaryMap grps ; for(;lit != tr->mItems.end(); ++lit) { @@ -3803,7 +3802,7 @@ void RsGxsNetService::handleRecvSyncGroup(RsNxsSyncGrpReqItem *item) return; } - RsGxsMetaDataTemporaryMap grp; + RsGxsGrpMetaTemporaryMap grp; mDataStore->retrieveGxsGrpMetaData(grp); #ifdef NXS_NET_DEBUG_0 @@ -4089,7 +4088,7 @@ void RsGxsNetService::handleRecvSyncMessage(RsNxsSyncMsgReqItem *item,bool item_ return; } - RsGxsMetaDataTemporaryMap grpMetas; + RsGxsGrpMetaTemporaryMap grpMetas; grpMetas[item->grpId] = NULL; mDataStore->retrieveGxsGrpMetaData(grpMetas); @@ -4628,7 +4627,7 @@ void RsGxsNetService::sharePublishKeysPending() // Get the meta data for this group Id // - RsGxsMetaDataTemporaryMap grpMetaMap; + RsGxsGrpMetaTemporaryMap grpMetaMap; grpMetaMap[mit->first] = NULL; mDataStore->retrieveGxsGrpMetaData(grpMetaMap); @@ -4712,7 +4711,7 @@ void RsGxsNetService::handleRecvPublishKeys(RsNxsGroupPublishKeyItem *item) // Get the meta data for this group Id // - RsGxsMetaDataTemporaryMap grpMetaMap; + RsGxsGrpMetaTemporaryMap grpMetaMap; grpMetaMap[item->grpId] = NULL; mDataStore->retrieveGxsGrpMetaData(grpMetaMap); diff --git a/libretroshare/src/gxs/rsgxsutil.h b/libretroshare/src/gxs/rsgxsutil.h index ea94fedcb..f55c7c147 100644 --- a/libretroshare/src/gxs/rsgxsutil.h +++ b/libretroshare/src/gxs/rsgxsutil.h @@ -51,37 +51,37 @@ void freeAndClearContainerResource(Container container) // temporary holds a map of pointers to class T, and destroys all pointers on delete. -template -class RsGxsMetaDataTemporaryMap: public std::map +template +class t_RsGxsGenericDataTemporaryMap: public std::map { public: - virtual ~RsGxsMetaDataTemporaryMap() + virtual ~t_RsGxsGenericDataTemporaryMap() { clear() ; } virtual void clear() { - for(typename RsGxsMetaDataTemporaryMap::iterator it = this->begin();it!=this->end();++it) + for(typename t_RsGxsGenericDataTemporaryMap::iterator it = this->begin();it!=this->end();++it) if(it->second != NULL) delete it->second ; - std::map::clear() ; + std::map::clear() ; } }; template -class RsGxsMetaDataTemporaryMapVector: public std::map > +class t_RsGxsGenericDataTemporaryMapVector: public std::map > { public: - virtual ~RsGxsMetaDataTemporaryMapVector() + virtual ~t_RsGxsGenericDataTemporaryMapVector() { clear() ; } virtual void clear() { - for(typename RsGxsMetaDataTemporaryMapVector::iterator it = this->begin();it!=this->end();++it) + for(typename t_RsGxsGenericDataTemporaryMapVector::iterator it = this->begin();it!=this->end();++it) { for(uint32_t i=0;isecond.size();++i) delete it->second[i] ; @@ -92,6 +92,13 @@ public: std::map >::clear() ; } }; + +typedef t_RsGxsGenericDataTemporaryMap RsGxsGrpMetaTemporaryMap; +typedef t_RsGxsGenericDataTemporaryMap RsNxsGrpDataTemporaryMap; + +typedef t_RsGxsGenericDataTemporaryMapVector RsGxsMsgMetaTemporaryMap ; +typedef t_RsGxsGenericDataTemporaryMapVector RsNxsMsgDataTemporaryMap ; + #ifdef UNUSED template class RsGxsMetaDataTemporaryMapVector: public std::vector diff --git a/libretroshare/src/rsitems/rsmsgitems.h b/libretroshare/src/rsitems/rsmsgitems.h index 527c43fdf..b35d9ea9f 100644 --- a/libretroshare/src/rsitems/rsmsgitems.h +++ b/libretroshare/src/rsitems/rsmsgitems.h @@ -238,7 +238,7 @@ class RsMsgParentId : public RsMessageItem class RsMsgSerialiser: public RsServiceSerializer { public: - RsMsgSerialiser(SerializationFlags flags) + RsMsgSerialiser(SerializationFlags flags = RsServiceSerializer::SERIALIZATION_FLAG_NONE) :RsServiceSerializer(RS_SERVICE_TYPE_MSG,RsGenericSerializer::FORMAT_BINARY,flags){} virtual ~RsMsgSerialiser() {} diff --git a/libretroshare/src/services/p3gxscircles.cc b/libretroshare/src/services/p3gxscircles.cc index 2e2796747..5e27e378d 100644 --- a/libretroshare/src/services/p3gxscircles.cc +++ b/libretroshare/src/services/p3gxscircles.cc @@ -2047,7 +2047,7 @@ bool p3GxsCircles::processMembershipRequests(uint32_t token) #ifdef DEBUG_CIRCLES std::cerr << "Processing circle membership requests." << std::endl; #endif - RsGxsMetaDataTemporaryMapVector msgItems; + t_RsGxsGenericDataTemporaryMapVector msgItems; if(!RsGenExchange::getMsgData(token, msgItems)) { diff --git a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc index 5874c303f..b12063a27 100644 --- a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc +++ b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc @@ -39,8 +39,10 @@ void test_groupStoreAndRetrieve(){ std::map grps, grps_copy; RsNxsGrp* grp; RsGxsGrpMetaData* grpMeta; - for(int i = 0; i < nGrp; i++){ - std::pair p; + + for(int i = 0; i < nGrp; i++) + { + std::pair p; grp = new RsNxsGrp(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); grpMeta = new RsGxsGrpMetaData(); p.first = grp; @@ -64,8 +66,9 @@ void test_groupStoreAndRetrieve(){ grps.clear(); grps = grps_copy; - std::map gR; - std::map grpMetaR; + RsNxsGrpDataTemporaryMap gR; + RsGxsGrpMetaTemporaryMap grpMetaR; + dStore->retrieveNxsGrps(gR, false, false); dStore->retrieveGxsGrpMetaData(grpMetaR); @@ -119,12 +122,6 @@ void test_groupStoreAndRetrieve(){ break; } - /* release resources */ - delete l_Meta; - delete r_Meta; - delete l; - delete r; - remove(grpId.toStdString().c_str()); } @@ -158,8 +155,8 @@ void test_messageStoresAndRetrieve() int nMsgs = rand()%120; GxsMsgReq req; - std::map VergrpId0, VergrpId1; - std::map VerMetagrpId0, VerMetagrpId1; + t_RsGxsGenericDataTemporaryMap VergrpId0, VergrpId1; + t_RsGxsGenericDataTemporaryMap VerMetagrpId0, VerMetagrpId1; for(int i=0; i msgResult ; //GxsMsgResult msgResult;. The temporary version cleans up itself. + t_RsGxsGenericDataTemporaryMapVector msgMetaResult ; - RsGxsMetaDataTemporaryMapVector msgMetaResult ; dStore->retrieveNxsMsgs(req, msgResult, false); - dStore->retrieveGxsMsgMetaData(req, msgMetaResult); // now look at result for grpId 1 From 70f72db0aa9993a3a05aff64902675a4e8f7fdd7 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 24 Jul 2017 18:20:21 +0200 Subject: [PATCH 05/14] fixed memory leak in GxsSecurity --- libretroshare/src/gxs/gxssecurity.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libretroshare/src/gxs/gxssecurity.cc b/libretroshare/src/gxs/gxssecurity.cc index 9c785c1a9..c321969d2 100644 --- a/libretroshare/src/gxs/gxssecurity.cc +++ b/libretroshare/src/gxs/gxssecurity.cc @@ -272,6 +272,8 @@ bool GxsSecurity::generateKeyPair(RsTlvPublicRSAKey& public_key,RsTlvPrivateRSAK RSA_generate_key_ex(rsa, 2048, ebn, NULL); RSA *rsa_pub = RSAPublicKey_dup(rsa); + BN_clear_free(ebn) ; + public_key.keyFlags = RSTLV_KEY_TYPE_PUBLIC_ONLY ; private_key.keyFlags = RSTLV_KEY_TYPE_FULL ; From 79779b8056d7bc565bead0c481986221c7ba3662 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 24 Jul 2017 18:45:24 +0200 Subject: [PATCH 06/14] fixed memory leaks in GxsSecurity --- libretroshare/src/gxs/gxssecurity.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libretroshare/src/gxs/gxssecurity.cc b/libretroshare/src/gxs/gxssecurity.cc index c321969d2..7d200ab86 100644 --- a/libretroshare/src/gxs/gxssecurity.cc +++ b/libretroshare/src/gxs/gxssecurity.cc @@ -582,6 +582,8 @@ bool GxsSecurity::encrypt(uint8_t *& out, uint32_t &outlen, const uint8_t *in, u // intialize context and send store encrypted cipher in ek if(!EVP_SealInit(ctx, EVP_aes_128_cbc(), &ek, &eklen, iv, &public_key, 1)) return false; + EVP_PKEY_free(public_key) ; + // now assign memory to out accounting for data, and cipher block size, key length, and key length val out = (uint8_t*)rs_malloc(inlen + cipher_block_size + size_net_ekl + eklen + EVP_MAX_IV_LENGTH) ; @@ -859,6 +861,7 @@ bool GxsSecurity::decrypt(uint8_t *& out, uint32_t & outlen, const uint8_t *in, std::cerr << "(EE) Cannot decrypt data. Most likely reason: private GXS key is missing." << std::endl; return false; } + EVP_PKEY_free(privateKey) ; if(inlen < (uint32_t)in_offset) { From ef24459c5e6da5e6ea7283aab4ed7cfdb98fc1d5 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 24 Jul 2017 18:48:13 +0200 Subject: [PATCH 07/14] fixed memory leak in gxssecurity test --- .../libretroshare/gxs/security/gxssecurity_test.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unittests/libretroshare/gxs/security/gxssecurity_test.cc b/tests/unittests/libretroshare/gxs/security/gxssecurity_test.cc index ac293f566..8faba96cd 100644 --- a/tests/unittests/libretroshare/gxs/security/gxssecurity_test.cc +++ b/tests/unittests/libretroshare/gxs/security/gxssecurity_test.cc @@ -63,7 +63,7 @@ TEST(libretroshare_gxs, GxsSecurity) // create some random data and sign it / verify the signature. uint32_t data_len = 1000 + RSRandom::random_u32()%100 ; - char *data = new char[data_len] ; + RsTemporaryMemory data(data_len) ; RSRandom::random_bytes((unsigned char *)data,data_len) ; @@ -71,8 +71,8 @@ TEST(libretroshare_gxs, GxsSecurity) RsTlvKeySignature signature ; - EXPECT_TRUE(GxsSecurity::getSignature(data,data_len,priv_key,signature) ); - EXPECT_TRUE(GxsSecurity::validateSignature(data,data_len,pub_key,signature) ); + EXPECT_TRUE(GxsSecurity::getSignature((char*)(unsigned char*)data,data_len,priv_key,signature) ); + EXPECT_TRUE(GxsSecurity::validateSignature((char*)(unsigned char*)data,data_len,pub_key,signature) ); std::cerr << " Signature: size=" << signature.signData.bin_len << ", Hash=" << RsDirUtil::sha1sum((const uint8_t*)signature.signData.bin_data,signature.signData.bin_len) << std::endl; @@ -95,6 +95,9 @@ TEST(libretroshare_gxs, GxsSecurity) // EXPECT_TRUE(data_len == outlen2) ; EXPECT_TRUE(!memcmp(data,out2,outlen2)) ; + + free(out2) ; + free(out) ; } From 461ccf3b84dfe257eeda51206d7832ce877a9ae2 Mon Sep 17 00:00:00 2001 From: csoler Date: Tue, 25 Jul 2017 00:07:53 +0200 Subject: [PATCH 08/14] removed aliasing in storeMessages/storeGroups, removed several memory leaks in unittests, improved auto-delete structures in rsgxsutil, added deletion for members of RsGenExchange, removed shared pointers from unittests (they cause a memory leak and are only used in unittests) --- libretroshare/src/gxs/rsdataservice.cc | 183 ++++++++---------- libretroshare/src/gxs/rsdataservice.h | 6 +- libretroshare/src/gxs/rsgds.h | 6 +- libretroshare/src/gxs/rsgenexchange.cc | 73 +++---- libretroshare/src/gxs/rsgenexchange.h | 2 +- libretroshare/src/gxs/rsgxsdataaccess.cc | 17 +- libretroshare/src/gxs/rsgxsdataaccess.h | 2 +- libretroshare/src/gxs/rsgxsnetservice.h | 5 +- libretroshare/src/gxs/rsgxsutil.h | 50 +++-- libretroshare/src/util/rssharedptr.h | 1 + .../gxs/data_service/rsdataservice_test.cc | 78 +++----- .../gen_exchange/genexchangetestservice.cc | 9 +- .../gxs/gen_exchange/genexchangetestservice.h | 2 + .../gxs/gen_exchange/rsgenexchange_test.cc | 4 + .../gxs/nxs_test/nxsgrpsync_test.cc | 4 +- .../gxs/nxs_test/nxsmsgsync_test.cc | 24 ++- .../gxs/nxs_test/nxsmsgsync_test.h | 1 + .../libretroshare/gxs/nxs_test/nxstesthub.cc | 63 +++--- .../libretroshare/gxs/nxs_test/nxstesthub.h | 12 +- .../gxs/nxs_test/rsgxsnetservice_test.cc | 12 +- .../serialiser/rsmsgitem_test.cc | 2 +- 21 files changed, 281 insertions(+), 275 deletions(-) diff --git a/libretroshare/src/gxs/rsdataservice.cc b/libretroshare/src/gxs/rsdataservice.cc index 8486e95d0..535aa3035 100644 --- a/libretroshare/src/gxs/rsdataservice.cc +++ b/libretroshare/src/gxs/rsdataservice.cc @@ -702,20 +702,20 @@ RsNxsMsg* RsDataService::locked_getMessage(RetroCursor &c) return NULL; } -int RsDataService::storeMessage(std::map &msg) +int RsDataService::storeMessage(const std::list& msg) { RsStackMutex stack(mDbMutex); - std::map::iterator mit = msg.begin(); - // start a transaction mDb->beginTransaction(); - for(; mit != msg.end(); ++mit) + for(std::list::const_iterator mit = msg.begin(); mit != msg.end(); ++mit) { - RsNxsMsg* msgPtr = mit->first; - RsGxsMsgMetaData* msgMetaPtr = mit->second; + RsNxsMsg* msgPtr = *mit; + RsGxsMsgMetaData* msgMetaPtr = msgPtr->metaData; + + assert(msgMetaPtr != NULL); #ifdef RS_DATA_SERVICE_DEBUG std::cerr << "RsDataService::storeMessage() "; @@ -790,16 +790,6 @@ int RsDataService::storeMessage(std::map &msg) // finish transaction bool ret = mDb->commitTransaction(); - for(mit = msg.begin(); mit != msg.end(); ++mit) - { - //TODO: API encourages aliasing, remove this abomination - if(mit->second != mit->first->metaData) - delete mit->second; - - delete mit->first; - ; - } - return ret; } @@ -811,104 +801,94 @@ bool RsDataService::validSize(RsNxsMsg* msg) const } -int RsDataService::storeGroup(std::map &grp) +int RsDataService::storeGroup(const std::list& grp) { RsStackMutex stack(mDbMutex); - std::map::iterator sit = grp.begin(); - // begin transaction mDb->beginTransaction(); - for(; sit != grp.end(); ++sit) - { + for(std::list::const_iterator sit = grp.begin();sit != grp.end(); ++sit) + { + RsNxsGrp* grpPtr = *sit; + RsGxsGrpMetaData* grpMetaPtr = grpPtr->metaData; - RsNxsGrp* grpPtr = sit->first; - RsGxsGrpMetaData* grpMetaPtr = sit->second; + assert(grpMetaPtr != NULL); - // if data is larger than max item size do not add - if(!validSize(grpPtr)) continue; + // if data is larger than max item size do not add + if(!validSize(grpPtr)) continue; #ifdef RS_DATA_SERVICE_DEBUG - std::cerr << "RsDataService::storeGroup() GrpId: " << grpPtr->grpId.toStdString(); - std::cerr << " CircleType: " << (uint32_t) grpMetaPtr->mCircleType; - std::cerr << " CircleId: " << grpMetaPtr->mCircleId.toStdString(); - std::cerr << std::endl; + std::cerr << "RsDataService::storeGroup() GrpId: " << grpPtr->grpId.toStdString(); + std::cerr << " CircleType: " << (uint32_t) grpMetaPtr->mCircleType; + std::cerr << " CircleId: " << grpMetaPtr->mCircleId.toStdString(); + std::cerr << std::endl; #endif - /*! - * STORE data, data len, - * grpId, flags, publish time stamp, identity, - * id signature, admin signatue, key set, last posting ts - * and meta data - **/ - ContentValue cv; + /*! + * STORE data, data len, + * grpId, flags, publish time stamp, identity, + * id signature, admin signatue, key set, last posting ts + * and meta data + **/ + ContentValue cv; - uint32_t dataLen = grpPtr->grp.TlvSize(); - char grpData[dataLen]; - uint32_t offset = 0; - grpPtr->grp.SetTlv(grpData, dataLen, &offset); - cv.put(KEY_NXS_DATA, dataLen, grpData); + uint32_t dataLen = grpPtr->grp.TlvSize(); + char grpData[dataLen]; + uint32_t offset = 0; + grpPtr->grp.SetTlv(grpData, dataLen, &offset); + cv.put(KEY_NXS_DATA, dataLen, grpData); - cv.put(KEY_NXS_DATA_LEN, (int32_t) dataLen); - cv.put(KEY_GRP_ID, grpPtr->grpId.toStdString()); - cv.put(KEY_GRP_NAME, grpMetaPtr->mGroupName); - cv.put(KEY_ORIG_GRP_ID, grpMetaPtr->mOrigGrpId.toStdString()); - cv.put(KEY_NXS_SERV_STRING, grpMetaPtr->mServiceString); - cv.put(KEY_NXS_FLAGS, (int32_t)grpMetaPtr->mGroupFlags); - cv.put(KEY_TIME_STAMP, (int32_t)grpMetaPtr->mPublishTs); - cv.put(KEY_GRP_SIGN_FLAGS, (int32_t)grpMetaPtr->mSignFlags); - cv.put(KEY_GRP_CIRCLE_ID, grpMetaPtr->mCircleId.toStdString()); - cv.put(KEY_GRP_CIRCLE_TYPE, (int32_t)grpMetaPtr->mCircleType); - cv.put(KEY_GRP_INTERNAL_CIRCLE, grpMetaPtr->mInternalCircle.toStdString()); - cv.put(KEY_GRP_ORIGINATOR, grpMetaPtr->mOriginator.toStdString()); - cv.put(KEY_GRP_AUTHEN_FLAGS, (int32_t)grpMetaPtr->mAuthenFlags); - cv.put(KEY_PARENT_GRP_ID, grpMetaPtr->mParentGrpId.toStdString()); - cv.put(KEY_NXS_HASH, grpMetaPtr->mHash.toStdString()); - cv.put(KEY_RECV_TS, (int32_t)grpMetaPtr->mRecvTS); - cv.put(KEY_GRP_REP_CUTOFF, (int32_t)grpMetaPtr->mReputationCutOff); - cv.put(KEY_NXS_IDENTITY, grpMetaPtr->mAuthorId.toStdString()); + cv.put(KEY_NXS_DATA_LEN, (int32_t) dataLen); + cv.put(KEY_GRP_ID, grpPtr->grpId.toStdString()); + cv.put(KEY_GRP_NAME, grpMetaPtr->mGroupName); + cv.put(KEY_ORIG_GRP_ID, grpMetaPtr->mOrigGrpId.toStdString()); + cv.put(KEY_NXS_SERV_STRING, grpMetaPtr->mServiceString); + cv.put(KEY_NXS_FLAGS, (int32_t)grpMetaPtr->mGroupFlags); + cv.put(KEY_TIME_STAMP, (int32_t)grpMetaPtr->mPublishTs); + cv.put(KEY_GRP_SIGN_FLAGS, (int32_t)grpMetaPtr->mSignFlags); + cv.put(KEY_GRP_CIRCLE_ID, grpMetaPtr->mCircleId.toStdString()); + cv.put(KEY_GRP_CIRCLE_TYPE, (int32_t)grpMetaPtr->mCircleType); + cv.put(KEY_GRP_INTERNAL_CIRCLE, grpMetaPtr->mInternalCircle.toStdString()); + cv.put(KEY_GRP_ORIGINATOR, grpMetaPtr->mOriginator.toStdString()); + cv.put(KEY_GRP_AUTHEN_FLAGS, (int32_t)grpMetaPtr->mAuthenFlags); + cv.put(KEY_PARENT_GRP_ID, grpMetaPtr->mParentGrpId.toStdString()); + cv.put(KEY_NXS_HASH, grpMetaPtr->mHash.toStdString()); + cv.put(KEY_RECV_TS, (int32_t)grpMetaPtr->mRecvTS); + cv.put(KEY_GRP_REP_CUTOFF, (int32_t)grpMetaPtr->mReputationCutOff); + cv.put(KEY_NXS_IDENTITY, grpMetaPtr->mAuthorId.toStdString()); - offset = 0; - char keySetData[grpMetaPtr->keys.TlvSize()]; - grpMetaPtr->keys.SetTlv(keySetData, grpMetaPtr->keys.TlvSize(), &offset); - cv.put(KEY_KEY_SET, grpMetaPtr->keys.TlvSize(), keySetData); + offset = 0; + char keySetData[grpMetaPtr->keys.TlvSize()]; + grpMetaPtr->keys.SetTlv(keySetData, grpMetaPtr->keys.TlvSize(), &offset); + cv.put(KEY_KEY_SET, grpMetaPtr->keys.TlvSize(), keySetData); - offset = 0; - char metaData[grpPtr->meta.TlvSize()]; - grpPtr->meta.SetTlv(metaData, grpPtr->meta.TlvSize(), &offset); - cv.put(KEY_NXS_META, grpPtr->meta.TlvSize(), metaData); + offset = 0; + char metaData[grpPtr->meta.TlvSize()]; + grpPtr->meta.SetTlv(metaData, grpPtr->meta.TlvSize(), &offset); + cv.put(KEY_NXS_META, grpPtr->meta.TlvSize(), metaData); - // local meta data - cv.put(KEY_GRP_SUBCR_FLAG, (int32_t)grpMetaPtr->mSubscribeFlags); - cv.put(KEY_GRP_POP, (int32_t)grpMetaPtr->mPop); - cv.put(KEY_MSG_COUNT, (int32_t)grpMetaPtr->mVisibleMsgCount); - cv.put(KEY_GRP_STATUS, (int32_t)grpMetaPtr->mGroupStatus); - cv.put(KEY_GRP_LAST_POST, (int32_t)grpMetaPtr->mLastPost); + // local meta data + cv.put(KEY_GRP_SUBCR_FLAG, (int32_t)grpMetaPtr->mSubscribeFlags); + cv.put(KEY_GRP_POP, (int32_t)grpMetaPtr->mPop); + cv.put(KEY_MSG_COUNT, (int32_t)grpMetaPtr->mVisibleMsgCount); + cv.put(KEY_GRP_STATUS, (int32_t)grpMetaPtr->mGroupStatus); + cv.put(KEY_GRP_LAST_POST, (int32_t)grpMetaPtr->mLastPost); - locked_clearGrpMetaCache(grpMetaPtr->mGroupId); + locked_clearGrpMetaCache(grpMetaPtr->mGroupId); - if (!mDb->sqlInsert(GRP_TABLE_NAME, "", cv)) - { - std::cerr << "RsDataService::storeGroup() sqlInsert Failed"; - std::cerr << std::endl; - std::cerr << "\t For GroupId: " << grpMetaPtr->mGroupId.toStdString(); - std::cerr << std::endl; - } - } + if (!mDb->sqlInsert(GRP_TABLE_NAME, "", cv)) + { + std::cerr << "RsDataService::storeGroup() sqlInsert Failed"; + std::cerr << std::endl; + std::cerr << "\t For GroupId: " << grpMetaPtr->mGroupId.toStdString(); + std::cerr << std::endl; + } + } // finish transaction bool ret = mDb->commitTransaction(); - for(sit = grp.begin(); sit != grp.end(); ++sit) - { - //TODO: API encourages aliasing, remove this abomination - if(sit->second != sit->first->metaData) - delete sit->second; - delete sit->first; - - } - return ret; } @@ -918,21 +898,21 @@ void RsDataService::locked_clearGrpMetaCache(const RsGxsGroupId& gid) mGrpMetaDataCache_ContainsAllDatabase = false; } -int RsDataService::updateGroup(std::map &grp) +int RsDataService::updateGroup(const std::list &grp) { RsStackMutex stack(mDbMutex); - std::map::iterator sit = grp.begin(); - // begin transaction mDb->beginTransaction(); - for(; sit != grp.end(); ++sit) + for( std::list::const_iterator sit = grp.begin(); sit != grp.end(); ++sit) { - RsNxsGrp* grpPtr = sit->first; - RsGxsGrpMetaData* grpMetaPtr = sit->second; + RsNxsGrp* grpPtr = *sit; + RsGxsGrpMetaData* grpMetaPtr = grpPtr->metaData; + + assert(grpMetaPtr != NULL); // if data is larger than max item size do not add if(!validSize(grpPtr)) continue; @@ -991,15 +971,6 @@ int RsDataService::updateGroup(std::map &grp) // finish transaction bool ret = mDb->commitTransaction(); - for(sit = grp.begin(); sit != grp.end(); ++sit) - { - //TODO: API encourages aliasing, remove this abomination - if(sit->second != sit->first->metaData) - delete sit->second; - delete sit->first; - - } - return ret; } diff --git a/libretroshare/src/gxs/rsdataservice.h b/libretroshare/src/gxs/rsdataservice.h index a48da407f..1e33747d6 100644 --- a/libretroshare/src/gxs/rsdataservice.h +++ b/libretroshare/src/gxs/rsdataservice.h @@ -127,21 +127,21 @@ public: * @param msg map of message and decoded meta data information * @return error code */ - int storeMessage(std::map& msg); + int storeMessage(const std::list& msg); /*! * Stores a list of groups in data store * @param grp map of group and decoded meta data * @return error code */ - int storeGroup(std::map& grp); + int storeGroup(const std::list& grp); /*! * Updates group entries in Db * @param grp map of group and decoded meta data * @return error code */ - int updateGroup(std::map& grsp); + int updateGroup(const std::list& grsp); /*! * @param metaData The meta data item to update diff --git a/libretroshare/src/gxs/rsgds.h b/libretroshare/src/gxs/rsgds.h index 2cb153056..33ed386c4 100644 --- a/libretroshare/src/gxs/rsgds.h +++ b/libretroshare/src/gxs/rsgds.h @@ -223,14 +223,14 @@ public: * @param msg map of message and decoded meta data information * @return error code */ - virtual int storeMessage(std::map& msgs) = 0; + virtual int storeMessage(const std::list& msgs) = 0; /*! * Stores a list of groups in data store * @param grp map of group and decoded meta data * @return error code */ - virtual int storeGroup(std::map& grsp) = 0; + virtual int storeGroup(const std::list& grsp) = 0; /*! @@ -238,7 +238,7 @@ public: * @param grp map of group and decoded meta data * @return error code */ - virtual int updateGroup(std::map& grsp) = 0; + virtual int updateGroup(const std::list& grsp) = 0; /*! * @param metaData diff --git a/libretroshare/src/gxs/rsgenexchange.cc b/libretroshare/src/gxs/rsgenexchange.cc index dd87cc10b..5be6fd3f1 100644 --- a/libretroshare/src/gxs/rsgenexchange.cc +++ b/libretroshare/src/gxs/rsgenexchange.cc @@ -117,6 +117,14 @@ RsGenExchange::~RsGenExchange() delete mDataStore; mDataStore = NULL; + for(uint32_t i=0;imsg, msg->metaData->mHash); mDataAccess->addMsgData(msg); + delete msg ; + msgChangeMap[grpId].push_back(msgId); delete[] metaDataBuff; @@ -2664,9 +2674,9 @@ void RsGenExchange::publishGrps() mDataAccess->updateGroupData(grp); else mDataAccess->addGroupData(grp); -#warning csoler: this is bad: addGroupData/updateGroupData actially deletes grp. But it may be used below? grp should be a class object and not deleted manually! - groups_to_subscribe.push_back(grpId) ; + delete grp ; + groups_to_subscribe.push_back(grpId) ; } else { @@ -2885,9 +2895,8 @@ void RsGenExchange::processRecvdMessages() std::vector::iterator vit = mReceivedMsgs.begin(); GxsMsgReq msgIds; - std::map msgs; - - std::map grpMetas; + RsNxsMsgDataTemporaryList msgs; + RsGxsGrpMetaTemporaryMap grpMetas; // coalesce group meta retrieval for performance for(; vit != mReceivedMsgs.end(); ++vit) @@ -2961,7 +2970,7 @@ void RsGenExchange::processRecvdMessages() if(validateReturn == VALIDATE_SUCCESS) { meta->mMsgStatus = GXS_SERV::GXS_MSG_STATUS_UNPROCESSED | GXS_SERV::GXS_MSG_STATUS_GUI_NEW | GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD; - msgs.insert(std::make_pair(msg, meta)); + msgs.push_back(msg); std::vector &msgv = msgIds[msg->grpId]; if (std::find(msgv.begin(), msgv.end(), msg->msgId) == msgv.end()) @@ -3038,15 +3047,9 @@ void RsGenExchange::processRecvdMessages() if(vit == mMsgPendingValidate.end()) mMsgPendingValidate.push_back(GxsPendingItem(msg, id,time(NULL))); -// else -// delete msg ; } } - // clean up resources from group meta retrieval - freeAndClearContainerResource, - RsGxsGrpMetaData*>(grpMetas); - if(!msgIds.empty()) { #ifdef GEN_EXCH_DEBUG @@ -3091,7 +3094,7 @@ void RsGenExchange::processRecvdGroups() std::vector existingGrpIds; std::list grpIds; - std::map grps; + RsNxsGrpDataTemporaryList grps; mDataStore->retrieveGroupIds(existingGrpIds); @@ -3145,7 +3148,7 @@ void RsGenExchange::processRecvdGroups() meta->mSubscribeFlags = GXS_SERV::GROUP_SUBSCRIBE_NOT_SUBSCRIBED; - grps.insert(std::make_pair(grp, meta)); + grps.push_back(grp); grpIds.push_back(grp->grpId); } else @@ -3250,7 +3253,9 @@ void RsGenExchange::performUpdateValidation() #endif vit = mGroupUpdates.begin(); - std::map grps; + + RsNxsGrpDataTemporaryList grps ; + for(; vit != mGroupUpdates.end(); ++vit) { GroupUpdate& gu = *vit; @@ -3265,7 +3270,7 @@ void RsGenExchange::performUpdateValidation() gu.newGrp->metaData->mSubscribeFlags = gu.oldGrpMeta->mSubscribeFlags ; - grps.insert(std::make_pair(gu.newGrp, gu.newGrp->metaData)); + grps.push_back(gu.newGrp); } else { @@ -3353,14 +3358,14 @@ void RsGenExchange::setGroupReputationCutOff(uint32_t& token, const RsGxsGroupId mGrpLocMetaMap.insert(std::make_pair(token, g)); } -void RsGenExchange::removeDeleteExistingMessages( RsGeneralDataService::MsgStoreMap& msgs, GxsMsgReq& msgIdsNotify) +void RsGenExchange::removeDeleteExistingMessages( std::list& msgs, GxsMsgReq& msgIdsNotify) { // first get grp ids of messages to be stored RsGxsGroupId::std_set mGrpIdsUnique; - for(RsGeneralDataService::MsgStoreMap::const_iterator cit = msgs.begin(); cit != msgs.end(); ++cit) - mGrpIdsUnique.insert(cit->second->mGroupId); + for(std::list::const_iterator cit = msgs.begin(); cit != msgs.end(); ++cit) + mGrpIdsUnique.insert((*cit)->metaData->mGroupId); //RsGxsGroupId::std_list grpIds(mGrpIdsUnique.begin(), mGrpIdsUnique.end()); //RsGxsGroupId::std_list::const_iterator it = grpIds.begin(); @@ -3381,13 +3386,10 @@ void RsGenExchange::removeDeleteExistingMessages( RsGeneralDataService::MsgStore #endif } - //RsGeneralDataService::MsgStoreMap::iterator cit2 = msgs.begin(); - RsGeneralDataService::MsgStoreMap filtered; - // now for each msg to be stored that exist in the retrieved msg/grp "index" delete and erase from map - for(RsGeneralDataService::MsgStoreMap::iterator cit2 = msgs.begin(); cit2 != msgs.end(); ++cit2) + for(std::list::iterator cit2 = msgs.begin(); cit2 != msgs.end(); ++cit2) { - const RsGxsMessageId::std_vector& msgIds = msgIdReq[cit2->second->mGroupId]; + const RsGxsMessageId::std_vector& msgIds = msgIdReq[(*cit2)->metaData->mGroupId]; #ifdef GEN_EXCH_DEBUG std::cerr << " grpid=" << cit2->second->mGroupId << ", msgid=" << cit2->second->mMsgId ; @@ -3395,38 +3397,27 @@ void RsGenExchange::removeDeleteExistingMessages( RsGeneralDataService::MsgStore // Avoid storing messages that are already in the database, as well as messages that are too old (or generally do not pass the database storage test) // - if(std::find(msgIds.begin(), msgIds.end(), cit2->second->mMsgId) == msgIds.end() && messagePublicationTest(*cit2->second)) - { - // passes tests, so add to filtered list - // - filtered.insert(*cit2); -#ifdef GEN_EXCH_DEBUG - std::cerr << " keeping " << cit2->second->mMsgId << std::endl; -#endif - } - else // remove message from list + if(std::find(msgIds.begin(), msgIds.end(), (*cit2)->metaData->mMsgId) != msgIds.end() || !messagePublicationTest( *(*cit2)->metaData)) { // msg exist in retrieved index - RsGxsMessageId::std_vector& notifyIds = msgIdsNotify[cit2->second->mGroupId]; - RsGxsMessageId::std_vector::iterator it2 = std::find(notifyIds.begin(), - notifyIds.end(), cit2->second->mMsgId); + RsGxsMessageId::std_vector& notifyIds = msgIdsNotify[ (*cit2)->metaData->mGroupId]; + RsGxsMessageId::std_vector::iterator it2 = std::find(notifyIds.begin(), notifyIds.end(), (*cit2)->metaData->mMsgId); if(it2 != notifyIds.end()) { notifyIds.erase(it2); if (notifyIds.empty()) { - msgIdsNotify.erase(cit2->second->mGroupId); + msgIdsNotify.erase( (*cit2)->metaData->mGroupId); } } #ifdef GEN_EXCH_DEBUG std::cerr << " discarding " << cit2->second->mMsgId << std::endl; #endif - delete cit2->first; + delete *cit2; + msgs.erase(cit2); // cit2->second will be deleted too in the destructor of cit2->first (RsNxsMsg) } } - - msgs = filtered; } diff --git a/libretroshare/src/gxs/rsgenexchange.h b/libretroshare/src/gxs/rsgenexchange.h index 093b342c9..78151cdae 100644 --- a/libretroshare/src/gxs/rsgenexchange.h +++ b/libretroshare/src/gxs/rsgenexchange.h @@ -850,7 +850,7 @@ private: * @param msgs messages to be filtered * @param msgIdsNotify message notification map to be filtered */ - void removeDeleteExistingMessages(RsGeneralDataService::MsgStoreMap& msgs, GxsMsgReq& msgIdsNotify); + void removeDeleteExistingMessages(std::list& msgs, GxsMsgReq& msgIdsNotify); RsMutex mGenMtx; RsGxsDataAccess* mDataAccess; diff --git a/libretroshare/src/gxs/rsgxsdataaccess.cc b/libretroshare/src/gxs/rsgxsdataaccess.cc index 8529b89f4..e010ffaf9 100644 --- a/libretroshare/src/gxs/rsgxsdataaccess.cc +++ b/libretroshare/src/gxs/rsgxsdataaccess.cc @@ -45,6 +45,11 @@ RsGxsDataAccess::RsGxsDataAccess(RsGeneralDataService* ds) : mDataStore(ds), mDataMutex("RsGxsDataAccess"), mNextToken(0) {} +RsGxsDataAccess::~RsGxsDataAccess() +{ + for(std::map::const_iterator it(mRequests.begin());it!=mRequests.end();++it) + delete it->second ; +} bool RsGxsDataAccess::requestGroupInfo(uint32_t &token, uint32_t ansType, const RsTokReqOptions &opts, const std::list &groupIds) { @@ -1803,8 +1808,8 @@ bool RsGxsDataAccess::addGroupData(RsNxsGrp* grp) { RsStackMutex stack(mDataMutex); - std::map grpM; - grpM.insert(std::make_pair(grp, grp->metaData)); + std::list grpM; + grpM.push_back(grp); return mDataStore->storeGroup(grpM); } @@ -1812,8 +1817,8 @@ bool RsGxsDataAccess::updateGroupData(RsNxsGrp* grp) { RsStackMutex stack(mDataMutex); - std::map grpM; - grpM.insert(std::make_pair(grp, grp->metaData)); + std::list grpM; + grpM.push_back(grp); return mDataStore->updateGroup(grpM); } @@ -1821,8 +1826,8 @@ bool RsGxsDataAccess::addMsgData(RsNxsMsg* msg) { RsStackMutex stack(mDataMutex); - std::map msgM; - msgM.insert(std::make_pair(msg, msg->metaData)); + std::list msgM; + msgM.push_back(msg); return mDataStore->storeMessage(msgM); } diff --git a/libretroshare/src/gxs/rsgxsdataaccess.h b/libretroshare/src/gxs/rsgxsdataaccess.h index d39823f81..04a785286 100644 --- a/libretroshare/src/gxs/rsgxsdataaccess.h +++ b/libretroshare/src/gxs/rsgxsdataaccess.h @@ -38,7 +38,7 @@ class RsGxsDataAccess : public RsTokenService { public: RsGxsDataAccess(RsGeneralDataService* ds); - virtual ~RsGxsDataAccess() { return ;} + virtual ~RsGxsDataAccess() ; public: diff --git a/libretroshare/src/gxs/rsgxsnetservice.h b/libretroshare/src/gxs/rsgxsnetservice.h index 369b88380..29c28e96e 100644 --- a/libretroshare/src/gxs/rsgxsnetservice.h +++ b/libretroshare/src/gxs/rsgxsnetservice.h @@ -71,13 +71,10 @@ class RsGroupNetworkStatsRecord * Incoming transaction are in 3 different states * 1. START 2. RECEIVING 3. END */ -class RsGxsNetService : public RsNetworkExchangeService, public p3ThreadedService, - public p3Config +class RsGxsNetService : public RsNetworkExchangeService, public p3ThreadedService, public p3Config { public: - typedef RsSharedPtr pointer; - static const uint32_t FRAGMENT_SIZE; /*! * only one observer is allowed diff --git a/libretroshare/src/gxs/rsgxsutil.h b/libretroshare/src/gxs/rsgxsutil.h index f55c7c147..a83d768ba 100644 --- a/libretroshare/src/gxs/rsgxsutil.h +++ b/libretroshare/src/gxs/rsgxsutil.h @@ -33,26 +33,19 @@ class RsGixs ; class RsGenExchange ; -/*! - * Handy function for cleaning out meta result containers - * @param container - */ -template -void freeAndClearContainerResource(Container container) -{ - typename Container::iterator meta_it = container.begin(); - - for(; meta_it != container.end(); ++meta_it) - if(meta_it->second != NULL) - delete meta_it->second; - - container.clear(); -} - // temporary holds a map of pointers to class T, and destroys all pointers on delete. +class non_copiable +{ +public: + non_copiable() {} +private: + non_copiable& operator=(const non_copiable&) { return *this ;} + non_copiable(const non_copiable&) {} +}; + template -class t_RsGxsGenericDataTemporaryMap: public std::map +class t_RsGxsGenericDataTemporaryMap: public std::map, public non_copiable { public: virtual ~t_RsGxsGenericDataTemporaryMap() @@ -71,7 +64,7 @@ public: }; template -class t_RsGxsGenericDataTemporaryMapVector: public std::map > +class t_RsGxsGenericDataTemporaryMapVector: public std::map >, public non_copiable { public: virtual ~t_RsGxsGenericDataTemporaryMapVector() @@ -93,12 +86,33 @@ public: } }; +template +class t_RsGxsGenericDataTemporaryList: public std::list, public non_copiable +{ +public: + virtual ~t_RsGxsGenericDataTemporaryList() + { + clear() ; + } + + virtual void clear() + { + for(typename t_RsGxsGenericDataTemporaryList::iterator it = this->begin();it!=this->end();++it) + delete *it; + + std::list::clear() ; + } +}; + typedef t_RsGxsGenericDataTemporaryMap RsGxsGrpMetaTemporaryMap; typedef t_RsGxsGenericDataTemporaryMap RsNxsGrpDataTemporaryMap; typedef t_RsGxsGenericDataTemporaryMapVector RsGxsMsgMetaTemporaryMap ; typedef t_RsGxsGenericDataTemporaryMapVector RsNxsMsgDataTemporaryMap ; +typedef t_RsGxsGenericDataTemporaryList RsNxsGrpDataTemporaryList ; +typedef t_RsGxsGenericDataTemporaryList RsNxsMsgDataTemporaryList ; + #ifdef UNUSED template class RsGxsMetaDataTemporaryMapVector: public std::vector diff --git a/libretroshare/src/util/rssharedptr.h b/libretroshare/src/util/rssharedptr.h index 70213952e..ca54f85b2 100644 --- a/libretroshare/src/util/rssharedptr.h +++ b/libretroshare/src/util/rssharedptr.h @@ -12,6 +12,7 @@ /*! * Not thread safe!! + * And also has a memory leak. Do not use (csoler, 24 Jul 2017). */ template class RsSharedPtr diff --git a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc index b12063a27..71d716ec2 100644 --- a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc +++ b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc @@ -36,49 +36,38 @@ void test_groupStoreAndRetrieve(){ setUp(); int nGrp = rand()%32; - std::map grps, grps_copy; + RsNxsGrpDataTemporaryList grps, grps_copy; RsNxsGrp* grp; RsGxsGrpMetaData* grpMeta; for(int i = 0; i < nGrp; i++) { std::pair p; - grp = new RsNxsGrp(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - grpMeta = new RsGxsGrpMetaData(); - p.first = grp; - p.second = grpMeta; - init_item(*grp); - init_item(grpMeta); - grpMeta->mGroupId = grp->grpId; - grps.insert(p); - RsNxsGrp* grp_copy = new RsNxsGrp(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - *grp_copy = *grp; - RsGxsGrpMetaData* grpMeta_copy = new RsGxsGrpMetaData(); - *grpMeta_copy = *grpMeta; - grps_copy.insert(std::make_pair(grp_copy, grpMeta_copy )); - grpMeta = NULL; - grp = NULL; - } + grp = new RsNxsGrp(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); + grpMeta = new RsGxsGrpMetaData(); + + init_item(*grp); + init_item(grpMeta); + + grpMeta->mGroupId = grp->grpId; + grp->metaData = grpMeta ; + + grps.push_back(grp); + } dStore->storeGroup(grps); - //use copy, a grps are deleted in store - grps.clear(); - grps = grps_copy; - RsNxsGrpDataTemporaryMap gR; RsGxsGrpMetaTemporaryMap grpMetaR; dStore->retrieveNxsGrps(gR, false, false); dStore->retrieveGxsGrpMetaData(grpMetaR); - std::map::iterator mit = grps.begin(); - bool grpMatch = true, grpMetaMatch = true; - for(; mit != grps.end(); mit++) + for( std::list::iterator mit = grps.begin(); mit != grps.end(); mit++) { - const RsGxsGroupId grpId = mit->first->grpId; + const RsGxsGroupId grpId = (*mit)->metaData->mGroupId; // check if it exists if(gR.find(grpId) == gR.end()) { @@ -86,8 +75,8 @@ void test_groupStoreAndRetrieve(){ break; } - RsNxsGrp *l = mit->first, - *r = gR[grpId]; + RsNxsGrp *l = (*mit); + RsNxsGrp *r = gR[grpId]; // assign transaction number // to right to as tn is not stored @@ -108,7 +97,7 @@ void test_groupStoreAndRetrieve(){ break; } - RsGxsGrpMetaData *l_Meta = mit->second, + RsGxsGrpMetaData *l_Meta = (*mit)->metaData, *r_Meta = grpMetaR[grpId]; // assign signSet and mGrpSize @@ -148,15 +137,18 @@ void test_messageStoresAndRetrieve() grpV.push_back(grpId0); grpV.push_back(grpId1); - std::map msgs; - std::map msgs_copy; + RsNxsMsgDataTemporaryList msgs; RsNxsMsg* msg = NULL; RsGxsMsgMetaData* msgMeta = NULL; int nMsgs = rand()%120; GxsMsgReq req; - t_RsGxsGenericDataTemporaryMap VergrpId0, VergrpId1; - t_RsGxsGenericDataTemporaryMap VerMetagrpId0, VerMetagrpId1; + // These ones are not in auto-delete structures because the data is deleted as part of the RsNxsMsg struct in the msgs list. + std::map VergrpId0 ; + std::map VergrpId1 ; + + std::map VerMetagrpId0; + std::map VerMetagrpId1; for(int i=0; imetaData = msgMeta ; + std::pair p(msg, msgMeta); int chosen = 0; if(rand()%50 > 24){ @@ -179,15 +174,9 @@ void test_messageStoresAndRetrieve() msgMeta->mMsgId = msg->msgId; msgMeta->mGroupId = msg->grpId = grpId; - RsNxsMsg* msg_copy = new RsNxsMsg(RS_SERVICE_TYPE_PLUGIN_SIMPLE_FORUM); - RsGxsMsgMetaData* msgMeta_copy = new RsGxsMsgMetaData(); - - *msg_copy = *msg; - *msgMeta_copy = *msgMeta; - // store msgs in map to use for verification - std::pair vP(msg->msgId, msg_copy); - std::pair vPmeta(msg->msgId, msgMeta_copy); + std::pair vP(msg->msgId, msg); + std::pair vPmeta(msg->msgId, msgMeta); if(!chosen) { @@ -201,19 +190,12 @@ void test_messageStoresAndRetrieve() } - - msg = NULL; - msgMeta = NULL; - - msgs.insert(p); - msgs_copy.insert(std::make_pair(msg_copy, msgMeta_copy)); + msgs.push_back(msg); } req[grpV[0]] = std::vector(); // assign empty list for other dStore->storeMessage(msgs); - msgs.clear(); - msgs = msgs_copy; // now retrieve msgs for comparison // first selective retrieval diff --git a/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.cc b/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.cc index d6c8134a9..add532fdd 100644 --- a/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.cc +++ b/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.cc @@ -1,12 +1,15 @@ #include "genexchangetestservice.h" -GenExchangeTestService::GenExchangeTestService(RsGeneralDataService *dataServ, RsNetworkExchangeService * netService, - RsGixs* gixs) - : RsGenExchange(dataServ, netService, new RsDummySerialiser(), RS_SERVICE_TYPE_DUMMY, gixs, 0) +GenExchangeTestService::GenExchangeTestService(RsGeneralDataService *dataServ, RsNetworkExchangeService * netService, RsGixs* gixs) + : RsGenExchange(dataServ, netService, mSerializer = new RsDummySerialiser(), RS_SERVICE_TYPE_DUMMY, gixs, 0) { } +GenExchangeTestService::~GenExchangeTestService() +{ + delete mSerializer ; +} RsServiceInfo GenExchangeTestService::getServiceInfo() { RsServiceInfo info; diff --git a/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.h b/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.h index e22386582..011ee6973 100644 --- a/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.h +++ b/tests/unittests/libretroshare/gxs/gen_exchange/genexchangetestservice.h @@ -11,6 +11,7 @@ class GenExchangeTestService : public RsGenExchange { public: GenExchangeTestService(RsGeneralDataService* dataServ, RsNetworkExchangeService* nxs, RsGixs* gixs); + virtual ~GenExchangeTestService(); void notifyChanges(std::vector& changes); @@ -104,6 +105,7 @@ public: void service_tick(); + RsSerialType *mSerializer ; }; #endif // GENEXCHANGETESTSERVICE_H diff --git a/tests/unittests/libretroshare/gxs/gen_exchange/rsgenexchange_test.cc b/tests/unittests/libretroshare/gxs/gen_exchange/rsgenexchange_test.cc index 3a402add5..32ffafe10 100644 --- a/tests/unittests/libretroshare/gxs/gen_exchange/rsgenexchange_test.cc +++ b/tests/unittests/libretroshare/gxs/gen_exchange/rsgenexchange_test.cc @@ -39,6 +39,8 @@ TEST(libretroshare_gxs, DISABLED_RsGenExchange) //GxsPublishMsgTest testMsgPublishing(&testService, dataStore); //testMsgPublishing.runTests(); + + //delete dataStore ; // deleted as a member of RsGenExchange } TEST(libretroshare_gxs, GetStats) @@ -54,4 +56,6 @@ TEST(libretroshare_gxs, GetStats) //GxsPublishMsgTest testMsgPublishing(&testService, dataStore); //testMsgPublishing.runTests(); + + //delete dataStore ; // deleted as a member of RsGenExchange } diff --git a/tests/unittests/libretroshare/gxs/nxs_test/nxsgrpsync_test.cc b/tests/unittests/libretroshare/gxs/nxs_test/nxsgrpsync_test.cc index 769eb4495..b926fe0e4 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/nxsgrpsync_test.cc +++ b/tests/unittests/libretroshare/gxs/nxs_test/nxsgrpsync_test.cc @@ -82,8 +82,8 @@ NxsGrpSync::NxsGrpSync(RsGcxs* circle, RsGixsReputation* reputation): RsGxsGroupId grpId = grp->grpId; - RsGeneralDataService::GrpStoreMap gsp; - gsp.insert(std::make_pair(grp, meta)); + RsNxsGrpDataTemporaryList gsp; + gsp.push_back(grp); mit->second->storeGroup(gsp); // the expected result is that each peer has the group of the others diff --git a/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.cc b/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.cc index f0b0e4dec..88cb4cf34 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.cc +++ b/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.cc @@ -17,6 +17,20 @@ using namespace rs_nxs_test; +rs_nxs_test::NxsMsgSync::~NxsMsgSync() +{ + for(std::map::const_iterator it(mNxsNetMgrs.begin());it!=mNxsNetMgrs.end();++it) + delete it->second ; + + for(DataMap::const_iterator it(mDataServices.begin());it!=mDataServices.end();++it) + delete it->second ; + + delete mRep ; + delete mCircles; + delete mPgpUtils; +} + + rs_nxs_test::NxsMsgSync::NxsMsgSync() : mPgpUtils(NULL), mServType(0) { int numPeers = 2; @@ -79,8 +93,8 @@ rs_nxs_test::NxsMsgSync::NxsMsgSync() // first store grp RsGeneralDataService* ds = mit->second; RsNxsGrp* grp_clone = grp->clone(); - RsGeneralDataService::GrpStoreMap gsp; - gsp.insert(std::make_pair(grp_clone, grp_clone->metaData)); + RsNxsGrpDataTemporaryList gsp; + gsp.push_back(grp_clone); ds->storeGroup(gsp); RsGxsGroupId grpId = grp->grpId; @@ -95,10 +109,12 @@ rs_nxs_test::NxsMsgSync::NxsMsgSync() msg->grpId = grp->grpId; RsGxsMsgMetaData* msgMeta = new RsGxsMsgMetaData(); init_item(msgMeta); + msg->metaData = msgMeta; msgMeta->mGroupId = grp->grpId; msgMeta->mMsgId = msg->msgId; - RsGeneralDataService::MsgStoreMap msm; - msm.insert(std::make_pair(msg , msgMeta)); + + RsNxsMsgDataTemporaryList msm; + msm.push_back(msg); RsGxsMessageId msgId = msg->msgId; ds->storeMessage(msm); diff --git a/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.h b/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.h index f81af79e9..ee1c03711 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.h +++ b/tests/unittests/libretroshare/gxs/nxs_test/nxsmsgsync_test.h @@ -17,6 +17,7 @@ namespace rs_nxs_test { public: NxsMsgSync(); + virtual ~NxsMsgSync(); void getPeers(std::list& peerIds); RsGeneralDataService* getDataService(const RsPeerId& peerId); RsNxsNetMgr* getDummyNetManager(const RsPeerId& peerId); diff --git a/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.cc b/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.cc index 36b288341..46dabe0a2 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.cc +++ b/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.cc @@ -60,7 +60,7 @@ private: }; -rs_nxs_test::NxsTestHub::NxsTestHub(NxsTestScenario::pointer testScenario) +rs_nxs_test::NxsTestHub::NxsTestHub(NxsTestScenario *testScenario) : mTestScenario(testScenario), mMtx("NxsTestHub Mutex") { std::list peers; @@ -73,31 +73,43 @@ rs_nxs_test::NxsTestHub::NxsTestHub(NxsTestScenario::pointer testScenario) for(; cit != peers.end(); cit++) { - RsGxsNetService::pointer ns = RsGxsNetService::pointer( - new RsGxsNetService( - mTestScenario->getServiceType(), - mTestScenario->getDataService(*cit), - mTestScenario->getDummyNetManager(*cit), - new NotifyWithPeerId(*cit, *this), - mTestScenario->getServiceInfo(), - mTestScenario->getDummyReputations(*cit), - mTestScenario->getDummyCircles(*cit), - NULL, - mTestScenario->getDummyPgpUtils(), - true - ) - ); + NotifyWithPeerId *noti = new NotifyWithPeerId(*cit, *this) ; - NxsTestHubConnection *connection = - new NxsTestHubConnection(*cit, this); + mNotifys.push_back(noti) ; + + RsGxsNetService *ns = new RsGxsNetService( + mTestScenario->getServiceType(), + mTestScenario->getDataService(*cit), + mTestScenario->getDummyNetManager(*cit), + noti, + mTestScenario->getServiceInfo(), + mTestScenario->getDummyReputations(*cit), + mTestScenario->getDummyCircles(*cit), + NULL, + mTestScenario->getDummyPgpUtils(), + true + ); + + NxsTestHubConnection *connection = new NxsTestHubConnection(*cit, this); ns->setServiceServer(connection); + mConnections.push_back(connection) ; + mPeerNxsMap.insert(std::make_pair(*cit, ns)); } } -rs_nxs_test::NxsTestHub::~NxsTestHub() { +rs_nxs_test::NxsTestHub::~NxsTestHub() +{ + for(PeerNxsMap::const_iterator it(mPeerNxsMap.begin());it!=mPeerNxsMap.end();++it) + delete it->second ; + + for(std::list::const_iterator it(mNotifys.begin());it!=mNotifys.end();++it) + delete *it ; + + for(std::list::const_iterator it(mConnections.begin());it!=mConnections.end();++it) + delete *it ; } @@ -137,7 +149,7 @@ void rs_nxs_test::NxsTestHub::notifyNewMessages(const RsPeerId& pid, { RS_STACK_MUTEX(mMtx); /***** MTX LOCKED *****/ - std::map toStore; + RsNxsMsgDataTemporaryList toStore; std::vector::iterator it = messages.begin(); for(; it != messages.end(); it++) { @@ -145,13 +157,17 @@ void rs_nxs_test::NxsTestHub::notifyNewMessages(const RsPeerId& pid, RsGxsMsgMetaData* meta = new RsGxsMsgMetaData(); // local meta is not touched by the deserialisation routine // have to initialise it + + msg->metaData = meta ; + meta->mMsgStatus = 0; meta->mMsgSize = 0; meta->mChildTs = 0; meta->recvTS = 0; meta->validated = false; meta->deserialise(msg->meta.bin_data, &(msg->meta.bin_len)); - toStore.insert(std::make_pair(msg, meta)); + + toStore.push_back(msg); } RsGeneralDataService* ds = mTestScenario->getDataService(pid); @@ -163,14 +179,15 @@ void rs_nxs_test::NxsTestHub::notifyNewGroups(const RsPeerId& pid, std::vector toStore; + RsNxsGrpDataTemporaryList toStore; std::vector::iterator it = groups.begin(); for(; it != groups.end(); it++) { RsNxsGrp* grp = *it; RsGxsGrpMetaData* meta = new RsGxsGrpMetaData(); + grp->metaData = meta ; meta->deserialise(grp->meta.bin_data, grp->meta.bin_len); - toStore.insert(std::make_pair(grp, meta)); + toStore.push_back(grp); } RsGeneralDataService* ds = mTestScenario->getDataService(pid); @@ -227,7 +244,7 @@ void rs_nxs_test::NxsTestHub::data_tick() // then tick net services for(; it != mPeerNxsMap.end(); it++) { - RsGxsNetService::pointer s = it->second; + RsGxsNetService *s = it->second; s->tick(); } diff --git a/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.h b/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.h index a37ff78e3..ee9524e7e 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.h +++ b/tests/unittests/libretroshare/gxs/nxs_test/nxstesthub.h @@ -11,7 +11,8 @@ // hence one could envision synchronising between an arbitrary number // of peers - +class NotifyWithPeerId; +class NxsTestHubConnection ; namespace rs_nxs_test { @@ -44,7 +45,7 @@ namespace rs_nxs_test * This constructs the test hub * for a give scenario in mind */ - NxsTestHub(NxsTestScenario::pointer testScenario); + NxsTestHub(NxsTestScenario* testScenario); /*! * This cleans up what ever testing resources are left @@ -101,13 +102,14 @@ namespace rs_nxs_test typedef std::pair PayLoad; - typedef std::map PeerNxsMap ; + typedef std::map PeerNxsMap ; - NxsTestScenario::pointer mTestScenario; + NxsTestScenario *mTestScenario; RsMutex mMtx; PeerNxsMap mPeerNxsMap; std::queue mPayLoad; - + std::list mNotifys; + std::list mConnections; }; } #endif // NXSTESTHUB_H diff --git a/tests/unittests/libretroshare/gxs/nxs_test/rsgxsnetservice_test.cc b/tests/unittests/libretroshare/gxs/nxs_test/rsgxsnetservice_test.cc index 47bfe8585..257b2de77 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/rsgxsnetservice_test.cc +++ b/tests/unittests/libretroshare/gxs/nxs_test/rsgxsnetservice_test.cc @@ -15,8 +15,7 @@ // disabled, because it fails after rebase to current master (did not fail in 2015, fails in 2016) TEST(libretroshare_gxs, DISABLED_gxs_grp_sync) { - rs_nxs_test::NxsTestScenario::pointer gsync_test = rs_nxs_test::NxsTestScenario::pointer( - new rs_nxs_test::NxsGrpSync()); + rs_nxs_test::NxsTestScenario *gsync_test = new rs_nxs_test::NxsGrpSync(); rs_nxs_test::NxsTestHub tHub(gsync_test); tHub.StartTest(); @@ -28,13 +27,13 @@ TEST(libretroshare_gxs, DISABLED_gxs_grp_sync) ASSERT_TRUE(tHub.testsPassed()); tHub.CleanUpTest(); + delete gsync_test ; } // disabled, not implemented (does currently the same as NxsGrpSync) TEST(libretroshare_gxs, DISABLED_gxs_grp_sync_delayed) { - rs_nxs_test::NxsTestScenario::pointer gsync_test = rs_nxs_test::NxsTestScenario::pointer( - new rs_nxs_test::NxsGrpSyncDelayed()); + rs_nxs_test::NxsTestScenario *gsync_test = new rs_nxs_test::NxsGrpSyncDelayed(); rs_nxs_test::NxsTestHub tHub(gsync_test); tHub.StartTest(); @@ -47,12 +46,12 @@ TEST(libretroshare_gxs, DISABLED_gxs_grp_sync_delayed) tHub.CleanUpTest(); + delete gsync_test ; } TEST(libretroshare_gxs, gxs_msg_sync) { - rs_nxs_test::NxsTestScenario::pointer gsync_test = rs_nxs_test::NxsTestScenario::pointer( - new rs_nxs_test::NxsMsgSync); + rs_nxs_test::NxsTestScenario *gsync_test = new rs_nxs_test::NxsMsgSync(); rs_nxs_test::NxsTestHub tHub(gsync_test); tHub.StartTest(); @@ -64,6 +63,7 @@ TEST(libretroshare_gxs, gxs_msg_sync) ASSERT_TRUE(tHub.testsPassed()); tHub.CleanUpTest(); + delete gsync_test ; } TEST(libretroshare_gxs, gxs_msg_sync_delayed) diff --git a/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc b/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc index a2b1369ad..838d367ad 100644 --- a/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsmsgitem_test.cc @@ -104,7 +104,7 @@ void init_item(RsChatAvatarItem& cai) { std::string image_data; randString(LARGE_STR, image_data); - cai.image_data = new unsigned char[image_data.size()]; + cai.image_data = (unsigned char*)malloc(image_data.size()); memcpy(cai.image_data, image_data.c_str(), image_data.size()); cai.image_size = image_data.size(); From 041f989f1cfcf5fce2d54ed4095008ba3738f673 Mon Sep 17 00:00:00 2001 From: csoler Date: Tue, 25 Jul 2017 21:21:24 +0200 Subject: [PATCH 09/14] rewrote processRecvdMessages() and processRecvdGroups() in RsGenExchange. Removed mReceivedGrps and mReceivedMsgs, cleaned-up terrible branching and memory management --- libretroshare/src/gxs/rsgenexchange.cc | 425 +++++++++++++++--- libretroshare/src/gxs/rsgenexchange.h | 11 +- .../serialiser/rsturtleitem_test.cc | 29 +- 3 files changed, 388 insertions(+), 77 deletions(-) diff --git a/libretroshare/src/gxs/rsgenexchange.cc b/libretroshare/src/gxs/rsgenexchange.cc index 5be6fd3f1..7919b8223 100644 --- a/libretroshare/src/gxs/rsgenexchange.cc +++ b/libretroshare/src/gxs/rsgenexchange.cc @@ -1339,11 +1339,16 @@ bool RsGenExchange::deserializeGroupData(unsigned char *data, uint32_t size, return false; } - mReceivedGrps.push_back( - GxsPendingItem( - nxs_grp, nxs_grp->grpId,time(NULL)) ); + if(mGrpPendingValidate.find(nxs_grp->grpId) != mGrpPendingValidate.end()) + { + std::cerr << "(WW) Group " << nxs_grp->grpId << " is already pending validation. Not adding again." << std::endl; + return true; + } - if(gId) *gId = nxs_grp->grpId; + if(gId) + *gId = nxs_grp->grpId; + + mGrpPendingValidate.insert(std::make_pair(nxs_grp->grpId, GxsPendingItem(nxs_grp, nxs_grp->grpId,time(NULL)))); return true; } @@ -1576,20 +1581,18 @@ void RsGenExchange::notifyNewGroups(std::vector &groups) for(; vit != groups.end(); ++vit) { RsNxsGrp* grp = *vit; - NxsGrpPendValidVect::iterator received = std::find(mReceivedGrps.begin(), - mReceivedGrps.end(), grp->grpId); + NxsGrpPendValidVect::iterator received = mGrpPendingValidate.find(grp->grpId); // drop group if you already have them // TODO: move this to nxs layer to save bandwidth - if(received == mReceivedGrps.end()) + if(received == mGrpPendingValidate.end()) { #ifdef GEN_EXCH_DEBUG std::cerr << "RsGenExchange::notifyNewGroups() Received GrpId: " << grp->grpId; std::cerr << std::endl; #endif - GxsPendingItem gpsi(grp, grp->grpId,time(NULL)); - mReceivedGrps.push_back(gpsi); + mGrpPendingValidate.insert(std::make_pair(grp->grpId, GxsPendingItem(grp, grp->grpId,time(NULL)))); } else { @@ -1604,37 +1607,36 @@ void RsGenExchange::notifyNewMessages(std::vector& messages) { RS_STACK_MUTEX(mGenMtx) ; - std::vector::iterator vit = messages.begin(); + // store these for tick() to pick them up - // store these for tick() to pick them up - for(; vit != messages.end(); ++vit) - { - RsNxsMsg* msg = *vit; - - NxsMsgPendingVect::iterator it = - std::find(mMsgPendingValidate.begin(), mMsgPendingValidate.end(), getMsgIdPair(*msg)); - - // if we have msg already just delete it - if(it == mMsgPendingValidate.end()) + for(uint32_t i=0;igrpId; - std::cerr << " MsgId: " << msg->msgId; - std::cerr << std::endl; -#endif + RsNxsMsg* msg = messages[i]; + NxsMsgPendingVect::iterator it = mMsgPendingValidate.find(msg->msgId) ; - mReceivedMsgs.push_back(msg); - } - else - { + // if we have msg already just delete it + if(it == mMsgPendingValidate.end()) + { #ifdef GEN_EXCH_DEBUG - std::cerr << " message is already in pending validation list. dropping." << std::endl; + std::cerr << "RsGenExchange::notifyNewMessages() Received Msg: "; + std::cerr << " GrpId: " << msg->grpId; + std::cerr << " MsgId: " << msg->msgId; + std::cerr << std::endl; #endif - delete msg; - } - } + RsGxsGrpMsgIdPair id; + id.first = msg->grpId; + id.second = msg->msgId; + mMsgPendingValidate.insert(std::make_pair(msg->msgId,GxsPendingItem(msg, id,time(NULL)))); + } + else + { +#ifdef GEN_EXCH_DEBUG + std::cerr << " message is already in pending validation list. dropping." << std::endl; +#endif + delete msg; + } + } } void RsGenExchange::notifyReceivePublishKey(const RsGxsGroupId &grpId) @@ -2854,6 +2856,188 @@ void RsGenExchange::computeHash(const RsTlvBinaryData& data, RsFileHash& hash) pHash.Complete(hash); } +void RsGenExchange::processRecvdMessages() +{ + std::list messages_to_reject ; + + { + RS_STACK_MUTEX(mGenMtx) ; + + time_t now = time(NULL); + +#ifdef GEN_EXCH_DEBUG + if(!mMsgPendingValidate.empty()) + std::cerr << "processing received messages" << std::endl; +#endif + // 1 - First, make sure items metadata is deserialised, clean old failed items, and collect the groups Ids we have to check + + RsGxsGrpMetaTemporaryMap grpMetas; + + for(NxsMsgPendingVect::iterator pend_it = mMsgPendingValidate.begin();pend_it != mMsgPendingValidate.end();) + { + GxsPendingItem& gpsi = pend_it->second; + RsNxsMsg *msg = gpsi.mItem ; + + if(msg->metaData == NULL) + { + RsGxsMsgMetaData* meta = new RsGxsMsgMetaData(); + + if(msg->meta.bin_len != 0 && meta->deserialise(msg->meta.bin_data, &(msg->meta.bin_len))) + msg->metaData = meta; + else + delete meta; + } + + bool accept_new_msg = msg->metaData != NULL && acceptNewMessage(msg->metaData,msg->msg.bin_len); + + if(!accept_new_msg && mNetService != NULL) + mNetService->rejectMessage(msg->metaData->mMsgId) ; // This prevents reloading the message again at next sync. + + if(!accept_new_msg || gpsi.mFirstTryTS + VALIDATE_MAX_WAITING_TIME < now) + { + std::cerr << "Pending validation grp=" << gpsi.mId.first << ", msg=" << gpsi.mId.second << ", has exceeded validation time limit. The author's key can probably not be obtained. This is unexpected." << std::endl; + + delete gpsi.mItem; + pend_it = mMsgPendingValidate.erase(pend_it); + } + else + { + grpMetas.insert(std::make_pair(pend_it->second.mItem->grpId, (RsGxsGrpMetaData*)NULL)); + ++pend_it; + } + } + + // 2 - Retrieve the metadata for the associated groups. + + mDataStore->retrieveGxsGrpMetaData(grpMetas); + + GxsMsgReq msgIds; + RsNxsMsgDataTemporaryList msgs_to_store; + +#ifdef GEN_EXCH_DEBUG + std::cerr << " updating received messages:" << std::endl; +#endif + + // 3 - Validate each message + + for(NxsMsgPendingVect::iterator pend_it = mMsgPendingValidate.begin();pend_it != mMsgPendingValidate.end();) + { + RsNxsMsg* msg = pend_it->second.mItem; + + // (cyril) Normally we should discard posts that are older than the sync request. But that causes a problem because + // RsGxsNetService requests posts to sync by chunks of 20. So if the 20 are discarded, they will be re-synced next time, and the sync process + // will indefinitly loop on the same 20 posts. Since the posts are there already, keeping them is the least problematique way to fix this problem. + // + // uint32_t max_sync_age = ( mNetService != NULL)?( mNetService->getSyncAge(msg->metaData->mGroupId)):RS_GXS_DEFAULT_MSG_REQ_PERIOD; + // + // if(max_sync_age != 0 && msg->metaData->mPublishTs + max_sync_age < time(NULL)) + // { + // std::cerr << "(WW) not validating message " << msg->metaData->mMsgId << " in group " << msg->metaData->mGroupId << " because it is older than synchronisation limit. This message was probably sent by a friend node that does not accept sync limits already." << std::endl; + // ok = false ; + // } + +#ifdef GEN_EXCH_DEBUG + std::cerr << " deserialised info: grp id=" << meta->mGroupId << ", msg id=" << meta->mMsgId ; +#endif + std::map::iterator mit = grpMetas.find(msg->grpId); + +#ifdef GEN_EXCH_DEBUG + std::cerr << " msg info : grp id=" << msg->grpId << ", msg id=" << msg->msgId << std::endl; +#endif + // validate msg + + if(mit == grpMetas.end()) + { + std::cerr << "RsGenExchange::processRecvdMessages(): impossible situation: grp meta " << msg->grpId << " not available." << std::endl; + ++pend_it ; + continue ; + } + + RsGxsGrpMetaData *grpMeta = mit->second; + + GxsSecurity::createPublicKeysFromPrivateKeys(grpMeta->keys); // make sure we have the public keys that correspond to the private ones, as it happens. Most of the time this call does nothing. + + int validateReturn = validateMsg(msg, grpMeta->mGroupFlags, grpMeta->mSignFlags, grpMeta->keys); + +#ifdef GEN_EXCH_DEBUG + std::cerr << " grpMeta.mSignFlags: " << std::hex << grpMeta->mSignFlags << std::dec << std::endl; + std::cerr << " grpMeta.mAuthFlags: " << std::hex << grpMeta->mAuthenFlags << std::dec << std::endl; + std::cerr << " message validation result: " << (int)validateReturn << std::endl; +#endif + + if(validateReturn == VALIDATE_SUCCESS) + { + msg->metaData->mMsgStatus = GXS_SERV::GXS_MSG_STATUS_UNPROCESSED | GXS_SERV::GXS_MSG_STATUS_GUI_NEW | GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD; + msgs_to_store.push_back(msg); + + std::vector &msgv = msgIds[msg->grpId]; + + if (std::find(msgv.begin(), msgv.end(), msg->msgId) == msgv.end()) + msgv.push_back(msg->msgId); + + computeHash(msg->msg, msg->metaData->mHash); + msg->metaData->recvTS = time(NULL); + +#ifdef GEN_EXCH_DEBUG + std::cerr << " new status flags: " << meta->mMsgStatus << std::endl; + std::cerr << " computed hash: " << meta->mHash << std::endl; + std::cerr << "Message received. Identity=" << msg->metaData->mAuthorId << ", from peer " << msg->PeerId() << std::endl; +#endif + + if(!msg->metaData->mAuthorId.isNull()) + mRoutingClues[msg->metaData->mAuthorId].insert(msg->PeerId()) ; + } + else if(validateReturn == VALIDATE_FAIL) + { + // In this case, we notify the network exchange service not to DL the message again, at least not yet. + +#ifdef GEN_EXCH_DEBUG + std::cerr << "Validation failed for message id " + << "msg->grpId: " << msg->grpId << ", msgId: " << msg->msgId << std::endl; +#endif + messages_to_reject.push_back(msg->msgId) ; + delete msg ; + } + else if(validateReturn == VALIDATE_FAIL_TRY_LATER) + { + ++pend_it ; + continue; + } + + // Remove the entry from mMsgPendingValidate, but do not delete msg since it's either pushed into msg_to_store or deleted in the FAIL case! + + NxsMsgPendingVect::iterator tmp = pend_it ; + ++tmp ; + mMsgPendingValidate.erase(pend_it) ; + pend_it = tmp ; + } + + if(!msgIds.empty()) + { +#ifdef GEN_EXCH_DEBUG + std::cerr << " removing existing and old messages from incoming list." << std::endl; +#endif + removeDeleteExistingMessages(msgs_to_store, msgIds); + +#ifdef GEN_EXCH_DEBUG + std::cerr << " storing remaining messages" << std::endl; +#endif + mDataStore->storeMessage(msgs_to_store); + + RsGxsMsgChange* c = new RsGxsMsgChange(RsGxsNotify::TYPE_RECEIVE, false); + c->msgChangeMap = msgIds; + mNotifications.push_back(c); + } + } + + // Done off-mutex to avoid cross deadlocks in the netservice that might call the RsGenExchange as an observer.. + + if(mNetService != NULL) + for(std::list::const_iterator it(messages_to_reject.begin());it!=messages_to_reject.end();++it) + mNetService->rejectMessage(*it) ; +} + +#ifdef OLD_VERSION void RsGenExchange::processRecvdMessages() { std::list messages_to_reject ; @@ -2913,14 +3097,16 @@ void RsGenExchange::processRecvdMessages() for(vit = mReceivedMsgs.begin(); vit != mReceivedMsgs.end(); ++vit) { RsNxsMsg* msg = *vit; - RsGxsMsgMetaData* meta = new RsGxsMsgMetaData(); - bool ok = false; + if(msg->metaData == NULL) + { + RsGxsMsgMetaData* meta = new RsGxsMsgMetaData(); - if(msg->meta.bin_len != 0) - ok = meta->deserialise(msg->meta.bin_data, &(msg->meta.bin_len)); - - msg->metaData = meta; + if(msg->meta.bin_len != 0 && meta->deserialise(msg->meta.bin_data, &(msg->meta.bin_len))) + msg->metaData = meta; + else + delete meta; + } // (cyril) Normally we should discard posts that are older than the sync request. But that causes a problem because // RsGxsNetService requests posts to sync by chunks of 20. So if the 20 are discarded, they will be re-synced next time, and the sync process @@ -2938,10 +3124,11 @@ void RsGenExchange::processRecvdMessages() std::cerr << " deserialised info: grp id=" << meta->mGroupId << ", msg id=" << meta->mMsgId ; #endif uint8_t validateReturn = VALIDATE_FAIL; - bool accept_new_msg = acceptNewMessage(meta,msg->msg.bin_len); - if(!accept_new_msg && mNetService != NULL) - mNetService->rejectMessage(meta->mMsgId) ; // This prevents reloading the message again at next sync. + bool accept_new_msg = acceptNewMessage(msg->metaData,msg->msg.bin_len); + + if(ok && !accept_new_msg && mNetService != NULL) + mNetService->rejectMessage(msg->metaData->mMsgId) ; // This prevents reloading the message again at next sync. if(ok && accept_new_msg) { @@ -3076,10 +3263,141 @@ void RsGenExchange::processRecvdMessages() for(std::list::const_iterator it(messages_to_reject.begin());it!=messages_to_reject.end();++it) mNetService->rejectMessage(*it) ; } +#endif bool RsGenExchange::acceptNewGroup(const RsGxsGrpMetaData* /*grpMeta*/ ) { return true; } bool RsGenExchange::acceptNewMessage(const RsGxsMsgMetaData* /*grpMeta*/,uint32_t /*size*/ ) { return true; } +void RsGenExchange::processRecvdGroups() +{ + RS_STACK_MUTEX(mGenMtx) ; + + if(mGrpPendingValidate.empty()) + return; + +#ifdef GEN_EXCH_DEBUG + std::cerr << "RsGenExchange::Processing received groups" << std::endl; +#endif + std::list grpIds; + RsNxsGrpDataTemporaryList grps_to_store; + + // 1 - retrieve the existing groups so as to check what's not new + std::vector existingGrpIds; + mDataStore->retrieveGroupIds(existingGrpIds); + + // 2 - go through each and every new group data and validate the signatures. + + for(NxsGrpPendValidVect::iterator vit = mGrpPendingValidate.begin(); vit != mGrpPendingValidate.end();) + { + GxsPendingItem& gpsi = vit->second; + RsNxsGrp* grp = gpsi.mItem; + +#ifdef GEN_EXCH_DEBUG + std::cerr << " processing validation for group " << meta->mGroupId << ", original attempt time: " << time(NULL) - gpsi.mFirstTryTS << " seconds ago" << std::endl; +#endif + if(grp->metaData == NULL) + { + RsGxsGrpMetaData* meta = new RsGxsGrpMetaData(); + + if(grp->meta.bin_len != 0 && meta->deserialise(grp->meta.bin_data, grp->meta.bin_len)) + grp->metaData = meta ; + else + delete meta ; + } + + // early deletion of group from the pending list if it's malformed, not accepted, or has been tried unsuccessfully for too long + + if(grp->metaData == NULL || !acceptNewGroup(grp->metaData) || gpsi.mFirstTryTS + VALIDATE_MAX_WAITING_TIME < time(NULL)) + { + NxsGrpPendValidVect::iterator tmp(vit) ; + ++tmp ; + delete grp ; + mGrpPendingValidate.erase(vit) ; + vit = tmp ; + continue; + } + + // group signature validation + + uint8_t ret = validateGrp(grp); + + if(ret == VALIDATE_SUCCESS) + { + grp->metaData->mGroupStatus = GXS_SERV::GXS_GRP_STATUS_UNPROCESSED | GXS_SERV::GXS_GRP_STATUS_UNREAD; + + computeHash(grp->grp, grp->metaData->mHash); + + // group has been validated. Let's notify the global router for the clue + + if(!grp->metaData->mAuthorId.isNull()) + { +#ifdef GEN_EXCH_DEBUG + std::cerr << "Group routage info: Identity=" << meta->mAuthorId << " from " << grp->PeerId() << std::endl; +#endif + mRoutingClues[grp->metaData->mAuthorId].insert(grp->PeerId()) ; + } + + // This has been moved here (as opposed to inside part for new groups below) because it is used to update the server TS when updates + // of grp metadata arrive. + + grp->metaData->mRecvTS = time(NULL); + + // now check if group already exists + + if(std::find(existingGrpIds.begin(), existingGrpIds.end(), grp->grpId) == existingGrpIds.end()) + { + grp->metaData->mOriginator = grp->PeerId(); + grp->metaData->mSubscribeFlags = GXS_SERV::GROUP_SUBSCRIBE_NOT_SUBSCRIBED; + + grps_to_store.push_back(grp); + grpIds.push_back(grp->grpId); + } + else + { + GroupUpdate update; + update.newGrp = grp; + mGroupUpdates.push_back(update); + } + } + else if(ret == VALIDATE_FAIL) + { +#ifdef GEN_EXCH_DEBUG + std::cerr << " failed to validate incoming meta, grpId: " << grp->grpId << ": wrong signature" << std::endl; +#endif + delete grp; + } + else if(ret == VALIDATE_FAIL_TRY_LATER) + { +#ifdef GEN_EXCH_DEBUG + std::cerr << " failed to validate incoming grp, trying again later. grpId: " << grp->grpId << std::endl; +#endif + ++vit ; + continue; + } + + // Erase entry from the list + + NxsGrpPendValidVect::iterator tmp(vit) ; + ++tmp ; + mGrpPendingValidate.erase(vit) ; + vit = tmp ; + } + + if(!grpIds.empty()) + { + RsGxsGroupChange* c = new RsGxsGroupChange(RsGxsNotify::TYPE_RECEIVE, false); + c->mGrpIdList = grpIds; + mNotifications.push_back(c); + mDataStore->storeGroup(grps_to_store); +#ifdef GEN_EXCH_DEBUG + std::cerr << " adding the following grp ids to notification: " << std::endl; + for(std::list::const_iterator it(grpIds.begin());it!=grpIds.end();++it) + std::cerr << " " << *it << std::endl; +#endif + } +} + +#ifdef OLD_VERSION void RsGenExchange::processRecvdGroups() { RS_STACK_MUTEX(mGenMtx) ; @@ -3102,27 +3420,31 @@ void RsGenExchange::processRecvdGroups() { GxsPendingItem& gpsi = *vit; RsNxsGrp* grp = gpsi.mItem; - RsGxsGrpMetaData* meta = new RsGxsGrpMetaData(); - bool deserialOk = false; - if(grp->meta.bin_len != 0) - deserialOk = meta->deserialise(grp->meta.bin_data, grp->meta.bin_len); + if(grp->metaData == NULL) + { + RsGxsGrpMetaData* meta = new RsGxsGrpMetaData(); + + if(grp->meta.bin_len != 0 && meta->deserialise(grp->meta.bin_data, grp->meta.bin_len)) + grp->metaData = meta ; + else + delete meta ; + } bool erase = true; - if(deserialOk && acceptNewGroup(meta)) + if(grp->metaData != NULL && acceptNewGroup(grp->metaData)) { #ifdef GEN_EXCH_DEBUG std::cerr << " processing validation for group " << meta->mGroupId << ", original attempt time: " << time(NULL) - gpsi.mFirstTryTS << " seconds ago" << std::endl; #endif - grp->metaData = meta; uint8_t ret = validateGrp(grp); if(ret == VALIDATE_SUCCESS) { - meta->mGroupStatus = GXS_SERV::GXS_GRP_STATUS_UNPROCESSED | GXS_SERV::GXS_GRP_STATUS_UNREAD; + grp->metaData->mGroupStatus = GXS_SERV::GXS_GRP_STATUS_UNPROCESSED | GXS_SERV::GXS_GRP_STATUS_UNREAD; - computeHash(grp->grp, meta->mHash); + computeHash(grp->grp, grp->metaData->mHash); // group has been validated. Let's notify the global router for the clue @@ -3215,6 +3537,7 @@ void RsGenExchange::processRecvdGroups() #endif } } +#endif void RsGenExchange::performUpdateValidation() { diff --git a/libretroshare/src/gxs/rsgenexchange.h b/libretroshare/src/gxs/rsgenexchange.h index 78151cdae..febfc5ff7 100644 --- a/libretroshare/src/gxs/rsgenexchange.h +++ b/libretroshare/src/gxs/rsgenexchange.h @@ -863,8 +863,8 @@ private: std::vector mReceivedMsgs; - typedef std::vector > NxsGrpPendValidVect; - NxsGrpPendValidVect mReceivedGrps; + typedef std::map > NxsGrpPendValidVect; + NxsGrpPendValidVect mGrpPendingValidate; std::vector mGrpsToPublish; typedef std::vector NxsGrpSignPendVect; @@ -885,11 +885,10 @@ private: /// authentication policy uint32_t mAuthenPolicy; - std::map > - mMsgPendingSign; + std::map > mMsgPendingSign; - std::vector > mMsgPendingValidate; - typedef std::vector > NxsMsgPendingVect; + typedef std::map > NxsMsgPendingVect; + NxsMsgPendingVect mMsgPendingValidate; bool mCleaning; time_t mLastClean; diff --git a/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc b/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc index 3590d519a..265d50e9c 100644 --- a/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc +++ b/tests/unittests/libretroshare/serialiser/rsturtleitem_test.cc @@ -42,8 +42,6 @@ RsSerialType* init_item(CompressedChunkMap& map) map._map.clear() ; for(uint32_t i=0;i<15;++i) map._map.push_back(rand()) ; - - return new RsTurtleSerialiser(); } bool operator==(const CompressedChunkMap& m1,const CompressedChunkMap& m2) { @@ -62,11 +60,10 @@ bool operator==(const RsTurtleFileMapRequestItem& it1,const RsTurtleFileMapReque return true ; } -RsSerialType* init_item(RsTurtleFileMapRequestItem& item) +void init_item(RsTurtleFileMapRequestItem& item) { item.direction = 1 ; item.tunnel_id = 0x4ff823e2 ; - return new RsTurtleSerialiser(); } bool operator==(const RsTurtleFileMapItem& it1,const RsTurtleFileMapItem& it2) { @@ -76,14 +73,13 @@ bool operator==(const RsTurtleFileMapItem& it1,const RsTurtleFileMapItem& it2) return true ; } -RsSerialType* init_item(RsTurtleFileMapItem& item) +void init_item(RsTurtleFileMapItem& item) { item.direction = 1 ; item.tunnel_id = 0xf48fe232 ; init_item(item.compressed_map) ; - return new RsTurtleSerialiser(); } -RsSerialType* init_item(RsTurtleFileDataItem& item) +void init_item(RsTurtleFileDataItem& item) { static const uint32_t S = 3456 ; item.tunnel_id = 0x33eef982 ; @@ -92,7 +88,6 @@ RsSerialType* init_item(RsTurtleFileDataItem& item) item.chunk_data = new unsigned char[S] ; for(uint32_t i=0;i Date: Tue, 25 Jul 2017 21:52:09 +0200 Subject: [PATCH 10/14] fixed memory leak in p3peermgr config load --- libretroshare/src/pqi/p3peermgr.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/libretroshare/src/pqi/p3peermgr.cc b/libretroshare/src/pqi/p3peermgr.cc index 993e1b137..68df7f01f 100644 --- a/libretroshare/src/pqi/p3peermgr.cc +++ b/libretroshare/src/pqi/p3peermgr.cc @@ -2320,6 +2320,7 @@ bool p3PeerMgrIMPL::loadList(std::list& load) std::cerr << "(II) Loaded group in new format. ID = " << info.id << std::endl; groupList[info.id] = info ; + delete *it ; continue; } RsPeerBandwidthLimitsItem *pblitem = dynamic_cast(*it) ; From 181a824d11fb6c7a5ab190b747dabcbf23d1210c Mon Sep 17 00:00:00 2001 From: csoler Date: Tue, 25 Jul 2017 21:56:20 +0200 Subject: [PATCH 11/14] fixed memory leak in BWGraphSource due to missing destructor --- retroshare-gui/src/gui/statistics/BWGraph.h | 1 + 1 file changed, 1 insertion(+) diff --git a/retroshare-gui/src/gui/statistics/BWGraph.h b/retroshare-gui/src/gui/statistics/BWGraph.h index 3e2c4b8b2..efa4a8a4e 100644 --- a/retroshare-gui/src/gui/statistics/BWGraph.h +++ b/retroshare-gui/src/gui/statistics/BWGraph.h @@ -23,6 +23,7 @@ public: }; BWGraphSource() ; + virtual ~BWGraphSource() {} enum { SELECTOR_TYPE_FRIEND=0x00, SELECTOR_TYPE_SERVICE=0x01 }; enum { GRAPH_TYPE_SINGLE=0x00, GRAPH_TYPE_ALL=0x01, GRAPH_TYPE_SUM=0x02 }; From 9881b616ac112f5c157bf8233c86ca9e9909fe50 Mon Sep 17 00:00:00 2001 From: csoler Date: Tue, 25 Jul 2017 22:06:18 +0200 Subject: [PATCH 12/14] cleanup code in RsGenExchange --- libretroshare/src/gxs/rsgenexchange.cc | 374 +------------------------ 1 file changed, 2 insertions(+), 372 deletions(-) diff --git a/libretroshare/src/gxs/rsgenexchange.cc b/libretroshare/src/gxs/rsgenexchange.cc index 7919b8223..7028d6295 100644 --- a/libretroshare/src/gxs/rsgenexchange.cc +++ b/libretroshare/src/gxs/rsgenexchange.cc @@ -2890,8 +2890,8 @@ void RsGenExchange::processRecvdMessages() bool accept_new_msg = msg->metaData != NULL && acceptNewMessage(msg->metaData,msg->msg.bin_len); - if(!accept_new_msg && mNetService != NULL) - mNetService->rejectMessage(msg->metaData->mMsgId) ; // This prevents reloading the message again at next sync. + if(!accept_new_msg) + messages_to_reject.push_back(msg->metaData->mMsgId); // This prevents reloading the message again at next sync. if(!accept_new_msg || gpsi.mFirstTryTS + VALIDATE_MAX_WAITING_TIME < now) { @@ -3037,234 +3037,6 @@ void RsGenExchange::processRecvdMessages() mNetService->rejectMessage(*it) ; } -#ifdef OLD_VERSION -void RsGenExchange::processRecvdMessages() -{ - std::list messages_to_reject ; - - { - RS_STACK_MUTEX(mGenMtx) ; - - time_t now = time(NULL); - -#ifdef GEN_EXCH_DEBUG - if(!mMsgPendingValidate.empty()) - std::cerr << "processing received messages" << std::endl; -#endif - NxsMsgPendingVect::iterator pend_it = mMsgPendingValidate.begin(); - - for(; pend_it != mMsgPendingValidate.end();) - { - GxsPendingItem& gpsi = *pend_it; - - if(gpsi.mFirstTryTS + VALIDATE_MAX_WAITING_TIME < now) - { - std::cerr << "Pending validation grp=" << gpsi.mId.first << ", msg=" << gpsi.mId.second << ", has exceeded validation time limit. The author's key can probably not be obtained. This is unexpected." << std::endl; - - delete gpsi.mItem; - pend_it = mMsgPendingValidate.erase(pend_it); - } - else - { -#ifdef GEN_EXCH_DEBUG - std::cerr << " movign to recvd." << std::endl; -#endif - mReceivedMsgs.push_back(gpsi.mItem); - ++pend_it; - } - } - - if(mReceivedMsgs.empty()) - return; - - std::vector::iterator vit = mReceivedMsgs.begin(); - GxsMsgReq msgIds; - RsNxsMsgDataTemporaryList msgs; - RsGxsGrpMetaTemporaryMap grpMetas; - - // coalesce group meta retrieval for performance - for(; vit != mReceivedMsgs.end(); ++vit) - { - RsNxsMsg* msg = *vit; - grpMetas.insert(std::make_pair(msg->grpId, (RsGxsGrpMetaData*)NULL)); - } - - mDataStore->retrieveGxsGrpMetaData(grpMetas); - -#ifdef GEN_EXCH_DEBUG - std::cerr << " updating received messages:" << std::endl; -#endif - for(vit = mReceivedMsgs.begin(); vit != mReceivedMsgs.end(); ++vit) - { - RsNxsMsg* msg = *vit; - - if(msg->metaData == NULL) - { - RsGxsMsgMetaData* meta = new RsGxsMsgMetaData(); - - if(msg->meta.bin_len != 0 && meta->deserialise(msg->meta.bin_data, &(msg->meta.bin_len))) - msg->metaData = meta; - else - delete meta; - } - - // (cyril) Normally we should discard posts that are older than the sync request. But that causes a problem because - // RsGxsNetService requests posts to sync by chunks of 20. So if the 20 are discarded, they will be re-synced next time, and the sync process - // will indefinitly loop on the same 20 posts. Since the posts are there already, keeping them is the least problematique way to fix this problem. - // - // uint32_t max_sync_age = ( mNetService != NULL)?( mNetService->getSyncAge(msg->metaData->mGroupId)):RS_GXS_DEFAULT_MSG_REQ_PERIOD; - // - // if(max_sync_age != 0 && msg->metaData->mPublishTs + max_sync_age < time(NULL)) - // { - // std::cerr << "(WW) not validating message " << msg->metaData->mMsgId << " in group " << msg->metaData->mGroupId << " because it is older than synchronisation limit. This message was probably sent by a friend node that does not accept sync limits already." << std::endl; - // ok = false ; - // } - -#ifdef GEN_EXCH_DEBUG - std::cerr << " deserialised info: grp id=" << meta->mGroupId << ", msg id=" << meta->mMsgId ; -#endif - uint8_t validateReturn = VALIDATE_FAIL; - - bool accept_new_msg = acceptNewMessage(msg->metaData,msg->msg.bin_len); - - if(ok && !accept_new_msg && mNetService != NULL) - mNetService->rejectMessage(msg->metaData->mMsgId) ; // This prevents reloading the message again at next sync. - - if(ok && accept_new_msg) - { - std::map::iterator mit = grpMetas.find(msg->grpId); - -#ifdef GEN_EXCH_DEBUG - std::cerr << " msg info : grp id=" << msg->grpId << ", msg id=" << msg->msgId << std::endl; -#endif - RsGxsGrpMetaData* grpMeta = NULL ; - - // validate msg - if(mit != grpMetas.end()) - { - grpMeta = mit->second; - GxsSecurity::createPublicKeysFromPrivateKeys(grpMeta->keys); // make sure we have the public keys that correspond to the private ones, as it happens. Most of the time this call does nothing. - - validateReturn = validateMsg(msg, grpMeta->mGroupFlags, grpMeta->mSignFlags, grpMeta->keys); - -#ifdef GEN_EXCH_DEBUG - std::cerr << " grpMeta.mSignFlags: " << std::hex << grpMeta->mSignFlags << std::dec << std::endl; - std::cerr << " grpMeta.mAuthFlags: " << std::hex << grpMeta->mAuthenFlags << std::dec << std::endl; - std::cerr << " message validation result: " << (int)validateReturn << std::endl; -#endif - } - - if(validateReturn == VALIDATE_SUCCESS) - { - meta->mMsgStatus = GXS_SERV::GXS_MSG_STATUS_UNPROCESSED | GXS_SERV::GXS_MSG_STATUS_GUI_NEW | GXS_SERV::GXS_MSG_STATUS_GUI_UNREAD; - msgs.push_back(msg); - - std::vector &msgv = msgIds[msg->grpId]; - if (std::find(msgv.begin(), msgv.end(), msg->msgId) == msgv.end()) - { - msgv.push_back(msg->msgId); - } - - NxsMsgPendingVect::iterator validated_entry = std::find(mMsgPendingValidate.begin(), mMsgPendingValidate.end(), - getMsgIdPair(*msg)); - - if(validated_entry != mMsgPendingValidate.end()) mMsgPendingValidate.erase(validated_entry); - - computeHash(msg->msg, meta->mHash); - meta->recvTS = time(NULL); -#ifdef GEN_EXCH_DEBUG - std::cerr << " new status flags: " << meta->mMsgStatus << std::endl; - std::cerr << " computed hash: " << meta->mHash << std::endl; - std::cerr << "Message received. Identity=" << msg->metaData->mAuthorId << ", from peer " << msg->PeerId() << std::endl; -#endif - - if(!msg->metaData->mAuthorId.isNull()) - mRoutingClues[msg->metaData->mAuthorId].insert(msg->PeerId()) ; - } - - if(validateReturn == VALIDATE_FAIL) - { - // In this case, we notify the network exchange service not to DL the message again, at least not yet. - -#ifdef GEN_EXCH_DEBUG - std::cerr << "Notifying the network service to not download this message again." << std::endl; -#endif - messages_to_reject.push_back(msg->msgId) ; - } - } - else - { -#ifdef GEN_EXCH_DEBUG - std::cerr << " deserialisation failed!" <grpId: " << msg->grpId << ", msgId: " << msg->msgId << std::endl; -#endif - - NxsMsgPendingVect::iterator failed_entry = std::find(mMsgPendingValidate.begin(), mMsgPendingValidate.end(), - getMsgIdPair(*msg)); - - if(failed_entry != mMsgPendingValidate.end()) mMsgPendingValidate.erase(failed_entry); - delete msg; - - - } - else if(validateReturn == VALIDATE_FAIL_TRY_LATER) - { - -#ifdef GEN_EXCH_DEBUG - std::cerr << "failed to validate msg, trying again: " - << "msg->grpId: " << msg->grpId << ", msgId: " << msg->msgId << std::endl; -#endif - - RsGxsGrpMsgIdPair id; - id.first = msg->grpId; - id.second = msg->msgId; - - // first check you haven't made too many attempts - - NxsMsgPendingVect::iterator vit = std::find(mMsgPendingValidate.begin(), mMsgPendingValidate.end(), id); - - if(vit == mMsgPendingValidate.end()) - mMsgPendingValidate.push_back(GxsPendingItem(msg, id,time(NULL))); - } - } - - if(!msgIds.empty()) - { -#ifdef GEN_EXCH_DEBUG - std::cerr << " removing existing and old messages from incoming list." << std::endl; -#endif - removeDeleteExistingMessages(msgs, msgIds); - -#ifdef GEN_EXCH_DEBUG - std::cerr << " storing remaining messages" << std::endl; -#endif - mDataStore->storeMessage(msgs); - - RsGxsMsgChange* c = new RsGxsMsgChange(RsGxsNotify::TYPE_RECEIVE, false); - c->msgChangeMap = msgIds; - mNotifications.push_back(c); - } - - mReceivedMsgs.clear(); - } - - // Done off-mutex to avoid cross deadlocks in the netservice that might call the RsGenExchange as an observer.. - - if(mNetService != NULL) - for(std::list::const_iterator it(messages_to_reject.begin());it!=messages_to_reject.end();++it) - mNetService->rejectMessage(*it) ; -} -#endif - bool RsGenExchange::acceptNewGroup(const RsGxsGrpMetaData* /*grpMeta*/ ) { return true; } bool RsGenExchange::acceptNewMessage(const RsGxsMsgMetaData* /*grpMeta*/,uint32_t /*size*/ ) { return true; } @@ -3397,148 +3169,6 @@ void RsGenExchange::processRecvdGroups() } } -#ifdef OLD_VERSION -void RsGenExchange::processRecvdGroups() -{ - RS_STACK_MUTEX(mGenMtx) ; - - if(mReceivedGrps.empty()) - return; - -#ifdef GEN_EXCH_DEBUG - std::cerr << "RsGenExchange::Processing received groups" << std::endl; -#endif - NxsGrpPendValidVect::iterator vit = mReceivedGrps.begin(); - std::vector existingGrpIds; - std::list grpIds; - - RsNxsGrpDataTemporaryList grps; - - mDataStore->retrieveGroupIds(existingGrpIds); - - while( vit != mReceivedGrps.end()) - { - GxsPendingItem& gpsi = *vit; - RsNxsGrp* grp = gpsi.mItem; - - if(grp->metaData == NULL) - { - RsGxsGrpMetaData* meta = new RsGxsGrpMetaData(); - - if(grp->meta.bin_len != 0 && meta->deserialise(grp->meta.bin_data, grp->meta.bin_len)) - grp->metaData = meta ; - else - delete meta ; - } - - bool erase = true; - - if(grp->metaData != NULL && acceptNewGroup(grp->metaData)) - { -#ifdef GEN_EXCH_DEBUG - std::cerr << " processing validation for group " << meta->mGroupId << ", original attempt time: " << time(NULL) - gpsi.mFirstTryTS << " seconds ago" << std::endl; -#endif - uint8_t ret = validateGrp(grp); - - if(ret == VALIDATE_SUCCESS) - { - grp->metaData->mGroupStatus = GXS_SERV::GXS_GRP_STATUS_UNPROCESSED | GXS_SERV::GXS_GRP_STATUS_UNREAD; - - computeHash(grp->grp, grp->metaData->mHash); - - // group has been validated. Let's notify the global router for the clue - - if(!meta->mAuthorId.isNull()) - { -#ifdef GEN_EXCH_DEBUG - std::cerr << "Group routage info: Identity=" << meta->mAuthorId << " from " << grp->PeerId() << std::endl; -#endif - - mRoutingClues[meta->mAuthorId].insert(grp->PeerId()) ; - } - - // This has been moved here (as opposed to inside part for new groups below) because it is used to update the server TS when updates - // of grp metadata arrive. - - meta->mRecvTS = time(NULL); - - // now check if group already existss - if(std::find(existingGrpIds.begin(), existingGrpIds.end(), grp->grpId) == existingGrpIds.end()) - { - //if(meta->mCircleType == GXS_CIRCLE_TYPE_YOUREYESONLY) - meta->mOriginator = grp->PeerId(); - - meta->mSubscribeFlags = GXS_SERV::GROUP_SUBSCRIBE_NOT_SUBSCRIBED; - - grps.push_back(grp); - grpIds.push_back(grp->grpId); - } - else - { - GroupUpdate update; - update.newGrp = grp; - mGroupUpdates.push_back(update); - } - erase = true; - } - else if(ret == VALIDATE_FAIL) - { -#ifdef GEN_EXCH_DEBUG - std::cerr << " failed to validate incoming meta, grpId: " << grp->grpId << ": wrong signature" << std::endl; -#endif - delete grp; - erase = true; - } - else if(ret == VALIDATE_FAIL_TRY_LATER) - { - -#ifdef GEN_EXCH_DEBUG - std::cerr << " failed to validate incoming grp, trying again. grpId: " << grp->grpId << std::endl; -#endif - - if(gpsi.mFirstTryTS + VALIDATE_MAX_WAITING_TIME < time(NULL)) - { -#ifdef GEN_EXCH_DEBUG - std::cerr << " validation time got group " << grp->grpId << " exceeded maximum. Will delete group " << std::endl; -#endif - delete grp; - erase = true; - } - else - erase = false; - } - } - else - { - if(!deserialOk) - std::cerr << "(EE) deserialise error in group meta data" << std::endl; - - delete grp; - delete meta; - erase = true; - } - - if(erase) - vit = mReceivedGrps.erase(vit); - else - ++vit; - } - - if(!grpIds.empty()) - { - RsGxsGroupChange* c = new RsGxsGroupChange(RsGxsNotify::TYPE_RECEIVE, false); - c->mGrpIdList = grpIds; - mNotifications.push_back(c); - mDataStore->storeGroup(grps); -#ifdef GEN_EXCH_DEBUG - std::cerr << " adding the following grp ids to notification: " << std::endl; - for(std::list::const_iterator it(grpIds.begin());it!=grpIds.end();++it) - std::cerr << " " << *it << std::endl; -#endif - } -} -#endif - void RsGenExchange::performUpdateValidation() { RS_STACK_MUTEX(mGenMtx) ; From b0483a0c38c54795a4b4c582396d622a4465dce7 Mon Sep 17 00:00:00 2001 From: csoler Date: Wed, 26 Jul 2017 10:23:18 +0200 Subject: [PATCH 13/14] fixed bug in removeDeleteExistingMsgs() --- libretroshare/src/gxs/rsgenexchange.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libretroshare/src/gxs/rsgenexchange.cc b/libretroshare/src/gxs/rsgenexchange.cc index 7028d6295..573ff5c6b 100644 --- a/libretroshare/src/gxs/rsgenexchange.cc +++ b/libretroshare/src/gxs/rsgenexchange.cc @@ -3340,7 +3340,7 @@ void RsGenExchange::removeDeleteExistingMessages( std::list& msgs, Gx } // now for each msg to be stored that exist in the retrieved msg/grp "index" delete and erase from map - for(std::list::iterator cit2 = msgs.begin(); cit2 != msgs.end(); ++cit2) + for(std::list::iterator cit2 = msgs.begin(); cit2 != msgs.end();) { const RsGxsMessageId::std_vector& msgIds = msgIdReq[(*cit2)->metaData->mGroupId]; @@ -3352,7 +3352,8 @@ void RsGenExchange::removeDeleteExistingMessages( std::list& msgs, Gx // if(std::find(msgIds.begin(), msgIds.end(), (*cit2)->metaData->mMsgId) != msgIds.end() || !messagePublicationTest( *(*cit2)->metaData)) { - // msg exist in retrieved index + // msg exist in retrieved index. We should use a std::set here instead of a vector. + RsGxsMessageId::std_vector& notifyIds = msgIdsNotify[ (*cit2)->metaData->mGroupId]; RsGxsMessageId::std_vector::iterator it2 = std::find(notifyIds.begin(), notifyIds.end(), (*cit2)->metaData->mMsgId); if(it2 != notifyIds.end()) @@ -3368,9 +3369,10 @@ void RsGenExchange::removeDeleteExistingMessages( std::list& msgs, Gx #endif delete *cit2; - msgs.erase(cit2); - // cit2->second will be deleted too in the destructor of cit2->first (RsNxsMsg) + cit2 = msgs.erase(cit2); } + else + ++cit2; } } From ea72c58d81c1e162be5227464c39af94e6602f45 Mon Sep 17 00:00:00 2001 From: csoler Date: Wed, 26 Jul 2017 12:27:38 +0200 Subject: [PATCH 14/14] fixed unnecessary () --- .../libretroshare/gxs/data_service/rsdataservice_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc index 71d716ec2..7f4e4b49c 100644 --- a/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc +++ b/tests/unittests/libretroshare/gxs/data_service/rsdataservice_test.cc @@ -75,7 +75,7 @@ void test_groupStoreAndRetrieve(){ break; } - RsNxsGrp *l = (*mit); + RsNxsGrp *l = *mit; RsNxsGrp *r = gR[grpId]; // assign transaction number