SoftingIndustrial / OPC-Classic-SDK

Softing OPC Classic SDK
https://industrial.softing.com/de/produkte/opc-ua-und-opc-classic-sdks.html
MIT License
53 stars 22 forks source link

IHC Bugfixes #7

Open mkentie opened 2 years ago

mkentie commented 2 years ago

-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.

mkentie commented 2 years ago

If you're interested I can also push our other changes, namely -Demo client compiles as 64-bit application -64-bit warning fixes -Ability to compile with VS2019 conformance mode (const char* fixes, two-phase-lookup in SOSrvComObject).

However these changes were only made to files we actually use (nothing SOA*), so it would leave the code base inconsistent.

FischerSeb commented 2 years ago

If you're interested I can also push our other changes, namely -Demo client compiles as 64-bit application -64-bit warning fixes -Ability to compile with VS2019 conformance mode (const char* fixes, two-phase-lookup in SOSrvComObject).

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.

mkentie commented 2 years ago

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 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: https://github.com/SoftingIndustrial/OPC-Classic-SDK/pull/7#pullrequestreview-908853148 You are receiving this because you were mentioned.

Message ID: @.***>

mrsuciu commented 2 years ago

@mkentie Good stuff. Can you please add the g_namespaceSync you mentioned, to keep the code cleaner?