diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyManagerComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyManagerComponent.cpp index 21f3b7d1c6..bd61e6ceed 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyManagerComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyManagerComponent.cpp @@ -63,23 +63,40 @@ namespace AzToolsFramework void PropertyManagerComponent::Deactivate() { + // Delete all remaining auto-delete or built-in handlers. for (auto it = m_builtInHandlers.begin(); it != m_builtInHandlers.end(); ++it) { - UnregisterPropertyType(*it); +#ifdef AZ_DEBUG_BUILD + // For debug builds, we'll take the extra time to delete each handler that we're deleting from m_Handlers. + // We loop through m_Handlers below to ensure that we don't have any other handlers still registered after + // we've deleted these. + AZStd::erase_if(m_Handlers, [it](const auto& item) { + auto const& [key, value] = item; + return (key == (*it)->GetHandlerName()) && (value == (*it)); + }); +#endif + delete *it; } m_builtInHandlers.clear(); - #ifdef _DEBUG +#ifdef AZ_DEBUG_BUILD + // Loop through all the remaining registered handlers (if any) and print out an error, as these all are probably memory + // leaks. UnregisterPropertyType should have been called on these already, and their pointers should have been deleted + // by the caller. auto it = m_Handlers.begin(); while (it != m_Handlers.end()) { AZ_Error("PropertyManager", false, "Property Handler 0x%08x is still registered during shutdown", it->first); ++it; } - #endif +#endif + + m_Handlers.clear(); + m_DefaultHandlers.clear(); + PropertyTypeRegistrationMessages::Bus::Handler::BusDisconnect(); } @@ -142,6 +159,16 @@ namespace AzToolsFramework } ++defaultIt; } + + if (pHandler->AutoDelete()) + { + m_builtInHandlers.erase(AZStd::remove(m_builtInHandlers.begin(), m_builtInHandlers.end(), pHandler), + m_builtInHandlers.end()); + AZ_Assert(false, + "Handlers with AutoDelete set should not call UnregisterPropertyType. To fix, do one of the following:\n" + " 1. Set AutoDelete to false in the handler, call UnregisterPropertyType, and the caller should delete the handler.\n" + " 2. Set AutoDelete to true in the handler and do NOT call UnregisterPropertyType or delete the handler."); + } } diff --git a/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.cpp b/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.cpp index 14f21c105d..d0f45be56d 100644 --- a/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.cpp +++ b/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.cpp @@ -25,8 +25,6 @@ namespace GradientSignal // GradientPreviewDataWidgetHandler // - GradientPreviewDataWidgetHandler* GradientPreviewDataWidgetHandler::s_instance = nullptr; - AZ::u32 GradientPreviewDataWidgetHandler::GetHandlerName() const { return AZ_CRC("GradientPreviewer", 0x1dbbba45); @@ -92,23 +90,18 @@ namespace GradientSignal { using namespace AzToolsFramework; - if (!s_instance) - { - s_instance = aznew GradientPreviewDataWidgetHandler(); - PropertyTypeRegistrationMessages::Bus::Broadcast(&PropertyTypeRegistrationMessages::Bus::Events::RegisterPropertyType, s_instance); - } + // Property handlers are set to auto-delete by default, which means that we're handing off ownership of the pointer to the + // PropertyManagerComponent, where it will get cleaned up on system shutdown. + auto propertyHandler = aznew GradientPreviewDataWidgetHandler(); + AZ_Assert(propertyHandler->AutoDelete(), + "GradientPreviewDataWidgetHandler is no longer set to auto-delete, it will leak memory."); + PropertyTypeRegistrationMessages::Bus::Broadcast( + &PropertyTypeRegistrationMessages::Bus::Events::RegisterPropertyType, propertyHandler); } void GradientPreviewDataWidgetHandler::Unregister() { - using namespace AzToolsFramework; - - if (s_instance) - { - PropertyTypeRegistrationMessages::Bus::Broadcast(&PropertyTypeRegistrationMessages::Bus::Events::UnregisterPropertyType, s_instance); - delete s_instance; - s_instance = nullptr; - } + // We don't need to call UnregisterPropertyType here because it's an autoDelete handler. } // diff --git a/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.h b/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.h index b5ee732ed7..cb5c0f8f20 100644 --- a/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.h +++ b/Gems/GradientSignal/Code/Source/UI/GradientPreviewDataWidget.h @@ -76,8 +76,5 @@ namespace GradientSignal static void Register(); static void Unregister(); - - private: - static GradientPreviewDataWidgetHandler* s_instance; }; } diff --git a/Gems/ScriptCanvas/Code/Editor/SystemComponent.cpp b/Gems/ScriptCanvas/Code/Editor/SystemComponent.cpp index 7d9cb176a3..60e8e5f23a 100644 --- a/Gems/ScriptCanvas/Code/Editor/SystemComponent.cpp +++ b/Gems/ScriptCanvas/Code/Editor/SystemComponent.cpp @@ -134,7 +134,7 @@ namespace ScriptCanvasEditor PopulateEditorCreatableTypes(); - m_propertyHandlers.emplace_back(AzToolsFramework::RegisterGenericComboBoxHandler()); + AzToolsFramework::RegisterGenericComboBoxHandler(); SystemRequestBus::Handler::BusConnect(); ScriptCanvasExecutionBus::Handler::BusConnect(); @@ -177,12 +177,6 @@ namespace ScriptCanvasEditor SystemRequestBus::Handler::BusDisconnect(); AzFramework::AssetCatalogEventBus::Handler::BusDisconnect(); - for (auto&& propertyHandler : m_propertyHandlers) - { - AzToolsFramework::PropertyTypeRegistrationMessages::Bus::Broadcast(&AzToolsFramework::PropertyTypeRegistrationMessages::UnregisterPropertyType, propertyHandler.get()); - } - m_propertyHandlers.clear(); - m_jobContext.reset(); m_jobManager.reset(); m_assetTracker.Deactivate(); diff --git a/Gems/ScriptCanvas/Code/Editor/SystemComponent.h b/Gems/ScriptCanvas/Code/Editor/SystemComponent.h index 07ea44e482..f2ebad38cd 100644 --- a/Gems/ScriptCanvas/Code/Editor/SystemComponent.h +++ b/Gems/ScriptCanvas/Code/Editor/SystemComponent.h @@ -147,7 +147,6 @@ namespace ScriptCanvasEditor AZStd::unique_ptr m_jobManager; AZStd::unique_ptr m_jobContext; - AZStd::vector> m_propertyHandlers; AZStd::unordered_set m_creatableTypes; AssetTracker m_assetTracker; diff --git a/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.cpp b/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.cpp index 5a4bc33550..f895517f8d 100644 --- a/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.cpp +++ b/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.cpp @@ -236,17 +236,11 @@ namespace ScriptEventsEditor moduleConfiguration->RegisterAssetHandler(); } - m_propertyHandlers.emplace_back(AzToolsFramework::RegisterGenericComboBoxHandler()); + AzToolsFramework::RegisterGenericComboBoxHandler(); } void ScriptEventEditorSystemComponent::Deactivate() { - for (auto&& propertyHandler : m_propertyHandlers) - { - AzToolsFramework::PropertyTypeRegistrationMessages::Bus::Broadcast(&AzToolsFramework::PropertyTypeRegistrationMessages::UnregisterPropertyType, propertyHandler.get()); - } - m_propertyHandlers.clear(); - using namespace ScriptEvents; ScriptEventsSystemComponentImpl* moduleConfiguration = nullptr; ScriptEventModuleConfigurationRequestBus::BroadcastResult(moduleConfiguration, &ScriptEventModuleConfigurationRequests::GetSystemComponentImpl); diff --git a/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.h b/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.h index d80dcffcf9..a5d34a9a9e 100644 --- a/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.h +++ b/Gems/ScriptEvents/Code/Source/Editor/ScriptEventsSystemEditorComponent.h @@ -90,8 +90,6 @@ namespace ScriptEventsEditor void Deactivate() override; //////////////////////////////////////////////////////////////////////// - AZStd::vector> m_propertyHandlers; - // Script Event Assets AZStd::unordered_map> m_scriptEvents; diff --git a/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.cpp b/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.cpp index 00849ed94b..17174fa132 100644 --- a/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.cpp +++ b/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.cpp @@ -58,14 +58,11 @@ namespace Vegetation void EditorVegetationSystemComponent::Activate() { // This is necessary for the m_spawnerType in Descriptor.cpp to display properly as a ComboBox - m_propertyHandler = aznew AzToolsFramework::GenericComboBoxHandler(); - AzToolsFramework::PropertyTypeRegistrationMessages::Bus::Broadcast(&AzToolsFramework::PropertyTypeRegistrationMessages::RegisterPropertyType, m_propertyHandler); + AzToolsFramework::RegisterGenericComboBoxHandler(); } void EditorVegetationSystemComponent::Deactivate() { - AzToolsFramework::PropertyTypeRegistrationMessages::Bus::Broadcast(&AzToolsFramework::PropertyTypeRegistrationMessages::UnregisterPropertyType, m_propertyHandler); - delete m_propertyHandler; } } diff --git a/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.h b/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.h index 76ac881f7c..2b45303672 100644 --- a/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.h +++ b/Gems/Vegetation/Code/Source/Editor/EditorVegetationSystemComponent.h @@ -35,9 +35,6 @@ namespace Vegetation void Activate() override; void Deactivate() override; - - private: - AzToolsFramework::PropertyHandlerBase* m_propertyHandler{ nullptr }; }; } // namespace Vegetation