Fix a crash when reloading AudioControlEditor controls (#2729)

* Fix a crash when reloading ACE controls data

The crash was due to destruction of xml_node that was held in a
unique_ptr.  Rapidxml has a very rudimentary memory allocation design,
so in most cases dynamic allocations aren't even made.  The memory_pool
does all the cleanup in its destructor, so having a unique_ptr run its
default_delete was causing the crash.

Signed-off-by: amzn-phist <52085794+amzn-phist@users.noreply.github.com>

* Fix numerical conversion warnings

Wwise source files needed a few fixes for the numerical conversion
warning changes that went in recently.

Signed-off-by: amzn-phist <52085794+amzn-phist@users.noreply.github.com>

Gems/AudioEngineWwise/Code/Source/Engine/FileIOHandler_wwise.cpp

@@ -69,7 +69,7 @@ namespace Audio
 
         AkDeviceSettings deviceSettings;
         AK::StreamMgr::GetDefaultDeviceSettings(deviceSettings);
-        deviceSettings.uIOMemorySize = poolSize;
+        deviceSettings.uIOMemorySize = aznumeric_cast<AkUInt32>(poolSize);
         deviceSettings.uSchedulerTypeFlags = AK_SCHEDULER_BLOCKING;
         Platform::SetThreadProperties(deviceSettings.threadProperties);
 
@@ -198,7 +198,7 @@ namespace Audio
         deviceDesc.bCanWrite = true;
         deviceDesc.deviceID = m_deviceID;
         AK_CHAR_TO_UTF16(deviceDesc.szDeviceName, "IO::IArchive", AZ_ARRAY_SIZE(deviceDesc.szDeviceName));
-        deviceDesc.uStringSize = AKPLATFORM::AkUtf16StrLen(deviceDesc.szDeviceName);
+        deviceDesc.uStringSize = aznumeric_cast<AkUInt32>(AKPLATFORM::AkUtf16StrLen(deviceDesc.szDeviceName));
     }
     
     AkUInt32 CBlockingDevice_wwise::GetDeviceData()
@@ -219,7 +219,7 @@ namespace Audio
 
         AkDeviceSettings deviceSettings;
         AK::StreamMgr::GetDefaultDeviceSettings(deviceSettings);
-        deviceSettings.uIOMemorySize = poolSize;
+        deviceSettings.uIOMemorySize = aznumeric_cast<AkUInt32>(poolSize);
         deviceSettings.uSchedulerTypeFlags = AK_SCHEDULER_DEFERRED_LINED_UP;
         Platform::SetThreadProperties(deviceSettings.threadProperties);
 
@@ -336,7 +336,7 @@ namespace Audio
         deviceDesc.bCanWrite = false;
         deviceDesc.deviceID = m_deviceID;
         AK_CHAR_TO_UTF16(deviceDesc.szDeviceName, "IO::IStreamer", AZ_ARRAY_SIZE(deviceDesc.szDeviceName));
-        deviceDesc.uStringSize = AKPLATFORM::AkUtf16StrLen(deviceDesc.szDeviceName);
+        deviceDesc.uStringSize = aznumeric_cast<AkUInt32>(AKPLATFORM::AkUtf16StrLen(deviceDesc.szDeviceName));
     }
 
     AkUInt32 CStreamingDevice_wwise::GetDeviceData()

Gems/AudioSystem/Code/Source/Editor/AudioControl.cpp

@@ -345,7 +345,7 @@ namespace AudioControls
         {
             for (auto& connectionNode : m_connectionNodes)
             {
-                if (TConnectionPtr connection = audioSystemImpl->CreateConnectionFromXMLNode(connectionNode.m_xmlNode.get(), m_type))
+                if (TConnectionPtr connection = audioSystemImpl->CreateConnectionFromXMLNode(connectionNode.m_xmlNode, m_type))
                 {
                     AddConnection(connection);
                     connectionNode.m_isValid = true;

Gems/AudioSystem/Code/Source/Editor/AudioControl.h

@@ -25,46 +25,48 @@ namespace AudioControls
     {
         SRawConnectionData(AZ::rapidxml::xml_node<char>* node, bool isValid)
         {
-            m_xmlNode = AZStd::move(DeepCopyNode(node));
+            m_xmlNode = DeepCopyNode(node);
             m_isValid = isValid;
         }
 
-        AZStd::unique_ptr<AZ::rapidxml::xml_node<char>> m_xmlNode{};
+        AZ::rapidxml::xml_node<char>* m_xmlNode = nullptr;
 
         // indicates if the connection is valid for the currently loaded middleware
         bool m_isValid{ false };
 
+    private:
         // Rapid XML provides a 'clone_node' utility that will copy an entire node tree,
         // but it only copies pointers of any strings in the node names and values.
-        // This causes problems with storing raw xml nodes as this class does because strings
-        // will be pointing into the memory pool of an xml document that has gone out of scope.
+        // This causes problems with storage of xml trees, as this class does, because strings
+        // will be pointing into an xml document's file buffer that has gone out of scope.
         // This function is a rewritten version of 'clone_node' that does the deep copy of strings
         // into the new destination tree.
-        [[nodiscard]] static AZStd::unique_ptr<AZ::rapidxml::xml_node<char>> DeepCopyNode(AZ::rapidxml::xml_node<char>* srcNode)
+        [[nodiscard]] AZ::rapidxml::xml_node<char>* DeepCopyNode(AZ::rapidxml::xml_node<char>* srcNode)
         {
-            AZStd::unique_ptr<AZ::rapidxml::xml_node<char>> destNode;
-            if (srcNode)
+            if (!srcNode)
             {
-                XmlAllocator& xmlAlloc(AudioControls::s_xmlAllocator);
-                destNode.reset(xmlAlloc.allocate_node(srcNode->type()));
+                return nullptr;
+            }
 
-                destNode->name(xmlAlloc.allocate_string(srcNode->name(), srcNode->name_size()), srcNode->name_size());
-                destNode->value(xmlAlloc.allocate_string(srcNode->value(), srcNode->value_size()), srcNode->value_size());
+            XmlAllocator& xmlAlloc(AudioControls::s_xmlAllocator);
+            AZ::rapidxml::xml_node<char>* destNode = xmlAlloc.allocate_node(srcNode->type());
 
-                for (AZ::rapidxml::xml_node<char>* child = srcNode->first_node(); child != nullptr; child = child->next_sibling())
-                {
-                    destNode->append_node(DeepCopyNode(child).release());
-                }
+            destNode->name(xmlAlloc.allocate_string(srcNode->name(), srcNode->name_size()), srcNode->name_size());
+            destNode->value(xmlAlloc.allocate_string(srcNode->value(), srcNode->value_size()), srcNode->value_size());
 
-                for (AZ::rapidxml::xml_attribute<char>* attr = srcNode->first_attribute(); attr != nullptr; attr = attr->next_attribute())
-                {
-                    destNode->append_attribute(xmlAlloc.allocate_attribute(
-                        xmlAlloc.allocate_string(attr->name(), attr->name_size()),
-                        xmlAlloc.allocate_string(attr->value(), attr->value_size()),
-                        attr->name_size(),
-                        attr->value_size()
-                    ));
-                }
+            for (AZ::rapidxml::xml_node<char>* child = srcNode->first_node(); child != nullptr; child = child->next_sibling())
+            {
+                destNode->append_node(DeepCopyNode(child));
+            }
+
+            for (AZ::rapidxml::xml_attribute<char>* attr = srcNode->first_attribute(); attr != nullptr; attr = attr->next_attribute())
+            {
+                destNode->append_attribute(xmlAlloc.allocate_attribute(
+                    xmlAlloc.allocate_string(attr->name(), attr->name_size()),
+                    xmlAlloc.allocate_string(attr->value(), attr->value_size()),
+                    attr->name_size(),
+                    attr->value_size()
+                ));
             }
 
             return destNode;

Gems/AudioSystem/Code/Source/Editor/AudioControlsLoader.cpp

@@ -475,7 +475,7 @@ namespace AudioControls
                     control->AddConnection(connection);
                 }
 
-                control->m_connectionNodes.push_back(SRawConnectionData(childNode, connection != nullptr));
+                control->m_connectionNodes.emplace_back(childNode, connection != nullptr);
 
                 childNode = childNode->next_sibling();
             }
@@ -517,7 +517,7 @@ namespace AudioControls
                     {
                         control->AddConnection(connection);
                     }
-                    control->m_connectionNodes.push_back(SRawConnectionData(connectionNode, connection != nullptr));
+                    control->m_connectionNodes.emplace_back(connectionNode, connection != nullptr);
                     connectionNode = connectionNode->next_sibling();
                 }
                 configGroupNode = configGroupNode->next_sibling();
@@ -534,7 +534,7 @@ namespace AudioControls
                 {
                     control->AddConnection(connection);
                 }
-                control->m_connectionNodes.push_back(SRawConnectionData(connectionNode, connection != nullptr));
+                control->m_connectionNodes.emplace_back(connectionNode, connection != nullptr);
                 connectionNode = connectionNode->next_sibling();
             }
         }
@@ -576,7 +576,7 @@ namespace AudioControls
 
         requestNode->append_node(valueNode);
 
-        childControl->m_connectionNodes.push_back(SRawConnectionData(requestNode, false));
+        childControl->m_connectionNodes.emplace_back(requestNode, false);
         return childControl;
     }
 

Gems/AudioSystem/Code/Source/Editor/AudioControlsWriter.cpp

@@ -356,8 +356,8 @@ namespace AudioControls
             {
                 if (!connectionNode.m_isValid)
                 {
-                    auto nodeCopy = SRawConnectionData::DeepCopyNode(connectionNode.m_xmlNode.get());
-                    node->append_node(nodeCopy.release());
+                    XmlAllocator& xmlAlloc(AudioControls::s_xmlAllocator);
+                    node->append_node(xmlAlloc.clone_node(connectionNode.m_xmlNode));
                 }
             }
 
@@ -371,7 +371,7 @@ namespace AudioControls
                         childNode != nullptr)
                     {
                         node->append_node(childNode);
-                        control->m_connectionNodes.push_back(SRawConnectionData(childNode, true));
+                        control->m_connectionNodes.emplace_back(childNode, true);
                     }
                 }
             }