From 27a535eaf472d7c891504af963b1c4460d59086a Mon Sep 17 00:00:00 2001 From: Steve Pham <82231385+spham-amzn@users.noreply.github.com> Date: Wed, 20 Oct 2021 14:34:30 -0700 Subject: [PATCH] Linux Fixes for Launching the Material Editor (#4808) - Prevent P4 thread to run if we cannot detect the P4 command to begin with - Add a trait to disable calling the parent ComponentApplication::Destroy(), instead calling _exit() to skip the module unloading on exit Signed-off-by: Steve Pham --- .../SourceControl/PerforceComponent.cpp | 23 +++++++++++++++---- .../SourceControl/PerforceComponent.h | 2 ++ .../AtomToolsFramework/Code/CMakeLists.txt | 4 ++++ .../Application/AtomToolsApplication.cpp | 4 ++++ .../Linux/AtomToolsFramework_Traits_Linux.h | 15 ++++++++++++ .../AtomToolsFramework_Traits_Platform.h | 10 ++++++++ .../Platform/Linux/platform_linux_files.cmake | 12 ++++++++++ .../Mac/AtomToolsFramework_Traits_Mac.h | 15 ++++++++++++ .../Mac/AtomToolsFramework_Traits_Platform.h | 10 ++++++++ .../Platform/Mac/platform_mac_files.cmake | 12 ++++++++++ .../AtomToolsFramework_Traits_Platform.h | 10 ++++++++ .../AtomToolsFramework_Traits_Windows.h | 15 ++++++++++++ .../Windows/platform_windows_files.cmake | 12 ++++++++++ 13 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Linux.h create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Platform.h create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/platform_linux_files.cmake create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Mac.h create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Platform.h create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/platform_mac_files.cmake create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Platform.h create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Windows.h create mode 100644 Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/platform_windows_files.cmake diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.cpp index 3bd12b1ad3..acb2cd4b1c 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.cpp @@ -20,6 +20,8 @@ #include #include +#include + namespace AzToolsFramework { namespace @@ -75,9 +77,17 @@ namespace AzToolsFramework m_resolveKey = true; m_testTrust = false; + // set up signals before we start thread. m_shutdownThreadSignal = false; - m_WorkerThread = AZStd::thread(AZStd::bind(&PerforceComponent::ThreadWorker, this)); + + // Check to see if the 'p4' command is available at the command line + int p4VersionExitCode = QProcess::execute("p4", QStringList{ "-V" }); + m_p4ApplicationDetected = (p4VersionExitCode == 0); + if (m_p4ApplicationDetected) + { + m_WorkerThread = AZStd::thread(AZStd::bind(&PerforceComponent::ThreadWorker, this)); + } SourceControlConnectionRequestBus::Handler::BusConnect(); SourceControlCommandBus::Handler::BusConnect(); @@ -88,10 +98,13 @@ namespace AzToolsFramework SourceControlCommandBus::Handler::BusDisconnect(); SourceControlConnectionRequestBus::Handler::BusDisconnect(); - m_shutdownThreadSignal = true; // tell the thread to die. - m_WorkerSemaphore.release(1); // wake up the thread so that it sees the signal - m_WorkerThread.join(); // wait for the thread to finish. - m_WorkerThread = AZStd::thread(); + if (m_p4ApplicationDetected) + { + m_shutdownThreadSignal = true; // tell the thread to die. + m_WorkerSemaphore.release(1); // wake up the thread so that it sees the signal + m_WorkerThread.join(); // wait for the thread to finish. + m_WorkerThread = AZStd::thread(); + } SetConnection(nullptr); } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.h index 858d9c0103..e5ccf7ad29 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/SourceControl/PerforceComponent.h @@ -260,5 +260,7 @@ namespace AzToolsFramework AZStd::atomic_bool m_validConnection; SourceControlState m_connectionState; + + bool m_p4ApplicationDetected { false }; }; } // namespace AzToolsFramework diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/CMakeLists.txt b/Gems/Atom/Tools/AtomToolsFramework/Code/CMakeLists.txt index ea5b65be7c..e64c9b80e7 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/CMakeLists.txt +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/CMakeLists.txt @@ -10,6 +10,8 @@ if(NOT PAL_TRAIT_BUILD_HOST_TOOLS) return() endif() +ly_get_list_relative_pal_filename(pal_source_dir ${CMAKE_CURRENT_LIST_DIR}/Source/Platform/${PAL_PLATFORM_NAME}) + ly_add_target( NAME AtomToolsFramework.Static STATIC NAMESPACE Gem @@ -18,9 +20,11 @@ ly_add_target( AUTORCC FILES_CMAKE atomtoolsframework_files.cmake + ${pal_source_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}_files.cmake INCLUDE_DIRECTORIES PRIVATE Source + ${pal_source_dir} PUBLIC Include BUILD_DEPENDENCIES diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Application/AtomToolsApplication.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Application/AtomToolsApplication.cpp index 3fae2ee7a3..58b07d8351 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Application/AtomToolsApplication.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Application/AtomToolsApplication.cpp @@ -216,7 +216,11 @@ namespace AtomToolsFramework AtomToolsMainWindowNotificationBus::Handler::BusDisconnect(); AzFramework::AssetSystemRequestBus::Broadcast(&AzFramework::AssetSystem::AssetSystemRequests::StartDisconnectingAssetProcessor); +#if AZ_TRAIT_ATOMTOOLSFRAMEWORK_SKIP_APP_DESTROY + _exit(0); +#else Base::Destroy(); +#endif } AZStd::vector AtomToolsApplication::GetCriticalAssetFilters() const diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Linux.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Linux.h new file mode 100644 index 0000000000..2f4c787fbe --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Linux.h @@ -0,0 +1,15 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +// On some platforms, there is an issue with environment variables that are removed before some objects are deallocated (during the process of +// ComponentApplication::Destroy). Until all of the shutdown issues are solved, the following trait will skip the parent ::Destroy() and exit +// the application as soon as possible if set to true. +// (Tracked by GHI - 4806) +#define AZ_TRAIT_ATOMTOOLSFRAMEWORK_SKIP_APP_DESTROY true + diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Platform.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Platform.h new file mode 100644 index 0000000000..8101a49a5a --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/AtomToolsFramework_Traits_Platform.h @@ -0,0 +1,10 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +#include diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/platform_linux_files.cmake b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/platform_linux_files.cmake new file mode 100644 index 0000000000..957ef8663e --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Linux/platform_linux_files.cmake @@ -0,0 +1,12 @@ +# +# Copyright (c) Contributors to the Open 3D Engine Project. +# For complete copyright and license terms please see the LICENSE at the root of this distribution. +# +# SPDX-License-Identifier: Apache-2.0 OR MIT +# +# + +set(FILES + AtomToolsFramework_Traits_Platform.h + AtomToolsFramework_Traits_Linux.h +) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Mac.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Mac.h new file mode 100644 index 0000000000..3ca246c797 --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Mac.h @@ -0,0 +1,15 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +// On some platforms, there is an issue with environment variables that are removed before some objects are deallocated (during the process of +// ComponentApplication::Destroy). Until all of the shutdown issues are solved, the following trait will skip the parent ::Destroy() and exit +// the application as soon as possible if set to true. +// (Tracked by GHI - 4806) +#define AZ_TRAIT_ATOMTOOLSFRAMEWORK_SKIP_APP_DESTROY false + diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Platform.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Platform.h new file mode 100644 index 0000000000..6d8c8d7e32 --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/AtomToolsFramework_Traits_Platform.h @@ -0,0 +1,10 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +#include diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/platform_mac_files.cmake b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/platform_mac_files.cmake new file mode 100644 index 0000000000..13cc6886ef --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Mac/platform_mac_files.cmake @@ -0,0 +1,12 @@ +# +# Copyright (c) Contributors to the Open 3D Engine Project. +# For complete copyright and license terms please see the LICENSE at the root of this distribution. +# +# SPDX-License-Identifier: Apache-2.0 OR MIT +# +# + +set(FILES + AtomToolsFramework_Traits_Platform.h + AtomToolsFramework_Traits_Mac.h +) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Platform.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Platform.h new file mode 100644 index 0000000000..ac82be7874 --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Platform.h @@ -0,0 +1,10 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +#include diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Windows.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Windows.h new file mode 100644 index 0000000000..3ca246c797 --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/AtomToolsFramework_Traits_Windows.h @@ -0,0 +1,15 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +// On some platforms, there is an issue with environment variables that are removed before some objects are deallocated (during the process of +// ComponentApplication::Destroy). Until all of the shutdown issues are solved, the following trait will skip the parent ::Destroy() and exit +// the application as soon as possible if set to true. +// (Tracked by GHI - 4806) +#define AZ_TRAIT_ATOMTOOLSFRAMEWORK_SKIP_APP_DESTROY false + diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/platform_windows_files.cmake b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/platform_windows_files.cmake new file mode 100644 index 0000000000..e2a2113bf9 --- /dev/null +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Platform/Windows/platform_windows_files.cmake @@ -0,0 +1,12 @@ +# +# Copyright (c) Contributors to the Open 3D Engine Project. +# For complete copyright and license terms please see the LICENSE at the root of this distribution. +# +# SPDX-License-Identifier: Apache-2.0 OR MIT +# +# + +set(FILES + AtomToolsFramework_Traits_Platform.h + AtomToolsFramework_Traits_Windows.h +)