Skip to content

Commit 0b37cb1

Browse files
authored
[src] Add more safe guards on memory corruption or double free in SofaContext DagNode parsing (#31)
* backup investigation work * unactive message handler. Too unstable for now * Add more safe guard on memory corruption or double free * use better conversion from std string to FString, seems much more stable * avoid reusing any UE component in several call, even if it should be a copy. To avoid potential problem of memory free * small potential fix
1 parent dc972c5 commit 0b37cb1

1 file changed

Lines changed: 45 additions & 28 deletions

File tree

Source/SofaUE5/Private/SofaContext.cpp

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
ASofaContext::ASofaContext()
3838
: Dt(0.02)
3939
, Gravity(0, -9.8, 0)
40-
, m_isMsgHandlerActivated(true)
40+
, m_isMsgHandlerActivated(false)
4141
, m_dllLoadStatus(0)
4242
, m_apiName("")
4343
, m_isInit(false)
@@ -69,7 +69,6 @@ void ASofaContext::OnConstruction(const FTransform& Transform)
6969
Super::OnConstruction(Transform);
7070

7171
#if WITH_EDITOR
72-
UE_LOG(SUnreal_log, Log, TEXT("########## ASofaContext:: BEfore createSofaContext: %s | %s ##########"), *this->GetName(), *LexToString(this->GetFlags()));
7372
if (m_sofaAPI == nullptr)
7473
{
7574
createSofaContext();
@@ -94,14 +93,15 @@ void ASofaContext::Destroyed()
9493
// First stop SOFA simulation
9594
m_sofaAPI->stop();
9695

97-
//if (m_isMsgHandlerActivated == true)
98-
// catchSofaMessages();
96+
if (m_isMsgHandlerActivated == true)
97+
catchSofaMessages();
9998

10099
if (m_log)
101100
UE_LOG(SUnreal_log, Log, TEXT("## ASofaContext::BeginDestroy: m_sofaAPI stopped"));
102101

103102
// Deactivate message handler
104-
m_sofaAPI->activateMessageHandler(false);
103+
if (m_isMsgHandlerActivated)
104+
m_sofaAPI->activateMessageHandler(false);
105105

106106
// Free SOFA context Ptr
107107
m_sofaAPI.Reset();
@@ -226,10 +226,12 @@ void ASofaContext::Tick( float DeltaTime )
226226

227227
//if (m_isMsgHandlerActivated == true)
228228
// catchSofaMessages();
229-
float value = this->GetGameTimeSinceCreation();
229+
230230
//UE_LOG(LogTemp, Warning, TEXT("## ASofaContext: Tick: %f %f"), value, stime);
231231
}
232-
232+
float value = this->GetGameTimeSinceCreation();
233+
UE_LOG(LogTemp, Warning, TEXT("## ASofaContext: Tick: %f"), value);
234+
233235
Super::Tick(DeltaTime);
234236
}
235237

@@ -238,8 +240,8 @@ void ASofaContext::Tick( float DeltaTime )
238240

239241
void ASofaContext::createSofaContext()
240242
{
241-
//if (m_log)
242-
UE_LOG(SUnreal_log, Log, TEXT("########## ASofaContext::createSofaContext: %s | %s ##########"), *this->GetName(), *LexToString(this->GetFlags()));
243+
if (m_log)
244+
UE_LOG(SUnreal_log, Log, TEXT("########## ASofaContext::createSofaContext: %s | %s ##########"), *this->GetName(), *LexToString(this->GetFlags()));
243245

244246
if (m_sofaAPI != nullptr) {
245247
UE_LOG(SUnreal_log, Error, TEXT("## ASofaContext::createSofaContext is called with a SofaAPI already created."));
@@ -253,7 +255,7 @@ void ASofaContext::createSofaContext()
253255
{
254256
m_sofaAPI = MakeShared<SofaAdvancePhysicsAPI>();
255257

256-
UE_LOG(SUnreal_log, Warning, TEXT("## ASofaDAGNode::loadComponents TEST 28"));
258+
UE_LOG(SUnreal_log, Warning, TEXT("## ASofaDAGNode::loadComponents TEST 29"));
257259

258260
if (m_sofaAPI == nullptr)
259261
{
@@ -262,7 +264,8 @@ void ASofaContext::createSofaContext()
262264
}
263265

264266
// activate message handler
265-
m_sofaAPI->activateMessageHandler(m_isMsgHandlerActivated);
267+
if (m_isMsgHandlerActivated)
268+
m_sofaAPI->activateMessageHandler(m_isMsgHandlerActivated);
266269

267270
// Test api Name
268271
m_apiName = m_sofaAPI->APIName();
@@ -367,7 +370,6 @@ void ASofaContext::loadDefaultPlugin()
367370

368371
// Start parsing scene loaded in SOFA
369372
// Create the actor of the scene:
370-
371373
void ASofaContext::loadNodeGraph()
372374
{
373375
if (m_sofaAPI == nullptr)
@@ -385,14 +387,17 @@ void ASofaContext::loadNodeGraph()
385387
return;
386388
}
387389

388-
FTransform SpawnTransform = FTransform::Identity;
390+
std::string parentNameId = "";
391+
std::string nodeUniqID = "";
392+
std::string nodeDisplayName = "";
393+
m_dagNodes.Reserve(nbrNode);
394+
395+
static FCriticalSection SofaAPILock;
396+
FScopeLock _(&SofaAPILock);
389397

390398
// First create all Nodes
391399
for (int nodeId = 0; nodeId < nbrNode; nodeId++)
392400
{
393-
std::string nodeUniqID = "";
394-
std::string nodeDisplayName = "";
395-
396401
int resNameId = m_sofaAPI->getDAGNodeAPIName_out(nodeId, nodeUniqID);
397402
int resDisplayName = m_sofaAPI->getDAGNodeDisplayName_out(nodeId, nodeDisplayName);
398403

@@ -401,46 +406,48 @@ void ASofaContext::loadNodeGraph()
401406
UE_LOG(SUnreal_log, Error, TEXT("## ASofaContext::loadNodeGraph: node name access return: %d | %d"), resNameId, resDisplayName);
402407
continue;
403408
}
404-
405-
FString fs_nodeUniqID(nodeUniqID.c_str());
406-
FString fs_nodeDisplayName(nodeDisplayName.c_str());
407-
408-
FActorSpawnParameters SpawnParams;
409-
SpawnParams.Name = MakeUniqueObjectName(World, ASofaDAGNode::StaticClass(), FName(*fs_nodeDisplayName));
410-
SpawnParams.Owner = this;
409+
410+
FString fs_nodeUniqID = UTF8_TO_TCHAR(nodeUniqID.c_str());
411+
FString fs_nodeDisplayName = UTF8_TO_TCHAR(nodeDisplayName.c_str());
411412

412413
ASofaDAGNode* dagNode = World->SpawnActorDeferred<ASofaDAGNode>(
413414
ASofaDAGNode::StaticClass(),
414-
SpawnTransform,
415+
FTransform::Identity,
415416
this,
416417
nullptr,
417418
ESpawnActorCollisionHandlingMethod::AlwaysSpawn
418419
);
419420

420421
if (dagNode != nullptr)
421422
{
422-
std::string parentNameId = "";
423423
int resParentNameId = m_sofaAPI->getDAGNodeParentAPIName_out(nodeUniqID, parentNameId);
424424
if (resParentNameId != 0)
425425
UE_LOG(SUnreal_log, Error, TEXT("## ASofaContext::loadNodeGraph: Getting parent name Id returns: %d"), resParentNameId);
426426

427-
FString fs_parentName(parentNameId.c_str());
427+
FString fs_parentName = UTF8_TO_TCHAR(parentNameId.c_str());
428428
dagNode->setUniqueNameID(fs_nodeUniqID);
429429
dagNode->setParentName(fs_parentName);
430430
dagNode->SetActorLabel(fs_nodeDisplayName);
431431

432432
if (m_log)
433433
UE_LOG(SUnreal_log, Log, TEXT("### ASofaDAGNode Created: %s | parent: %s | displayName: %s"), *fs_nodeUniqID, *fs_parentName, *fs_nodeDisplayName);
434434

435-
UGameplayStatics::FinishSpawningActor(dagNode, SpawnTransform);
435+
UGameplayStatics::FinishSpawningActor(dagNode, FTransform::Identity);
436436
m_dagNodes.Add(dagNode);
437437
}
438438
else
439439
{
440440
UE_LOG(SUnreal_log, Error, TEXT("## ASofaContext::loadNodeGraph: ASofaDAGNode actor not created: %s"), *fs_nodeDisplayName);
441441
}
442+
443+
// Set to null to be sure garbage collector do not mess it
444+
dagNode = nullptr;
442445
}
443446

447+
if (GEngine)
448+
{
449+
GEngine->ForceGarbageCollection(true); // Only in Editor builds
450+
}
444451

445452
// Reorder Node using Parent
446453
for (auto& WeakDagNode : m_dagNodes)
@@ -471,6 +478,11 @@ void ASofaContext::loadNodeGraph()
471478
if (m_isMsgHandlerActivated == true)
472479
catchSofaMessages();
473480

481+
if (GEngine)
482+
{
483+
GEngine->ForceGarbageCollection(true); // Only in Editor builds
484+
}
485+
474486
m_status++;
475487
}
476488

@@ -555,8 +567,13 @@ void ASofaContext::clearNodeGraph()
555567
void ASofaContext::catchSofaMessages()
556568
{
557569
int nbrMsgs = m_sofaAPI->getNbMessages();
558-
UE_LOG(SUnreal_log, Warning, TEXT("## ASofaContext::catchSofaMessages: nbr message: %d"), nbrMsgs);
559570

571+
572+
if (nbrMsgs == 0)
573+
return;
574+
575+
UE_LOG(SUnreal_log, Warning, TEXT("## ASofaContext::catchSofaMessages: nbr message: %d"), nbrMsgs);
576+
560577
for (int i = 0; i < nbrMsgs; ++i)
561578
{
562579
std::string rawMsg;

0 commit comments

Comments
 (0)