-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IHC Bugfixes #7
base: main
Are you sure you want to change the base?
IHC Bugfixes #7
Conversation
If you're interested I can also push our other changes, namely However these changes were only made to files we actually use (nothing SOA*), so it would leave the code base inconsistent. |
Thanks for the offer. But as you already pointed out it will not keep it simple here. |
@@ -674,6 +674,8 @@ SODaSNameSpaceRoot* SODaSEntry::getNameSpaceRoot( | |||
return NULL; | |||
} | |||
|
|||
SOCmnSingleLock<SOCmnSync> lock(&g_engineSync); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkentie Why must this lock be acquired here ? The only other place where it is used is in "SODaSEntry* getSODaSEntry(void) (line 122)" to guard the global g_engine.
When multiple threads simultaneously would try to write items, the lazy initialization of the namespace root would happen multiple times, resulting in a memory leak. However it could be that I just
re-used this lock, instead of adding a new lock member, because it was easy and wouldn't lead to issues. A seperate g_namespaceSync would have been more appropriate.
… @mrsuciu commented on this pull request.
> @@ -674,6 +674,8 @@ SODaSNameSpaceRoot* SODaSEntry::getNameSpaceRoot(
return NULL;
}
+ SOCmnSingleLock<SOCmnSync> lock(&g_engineSync);
@mkentie Why must this lock be acquired here ? The only other place where it is used is in "SODaSEntry* getSODaSEntry(void) (line 122)" to guard the global g_engine.
--
Reply to this email directly or view it on GitHub:
#7 (review)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
@mkentie Good stuff. Can you please add the g_namespaceSync you mentioned, to keep the code cleaner? |
-Wrong type of container in SOCmnList.cpp
-CoUninitialize not being called if initialized as COINIT_MULTITHREADED in SoCltEntry.
-Leaking of NameSpaceRoots during multi-threaded variable updates in SODaSEntry.cpp.