Fix recursive attempts to open the log file in the GameLauncher (#1114)

* Fix recursive attempts to open the log file in the GameLauncher

The AzFramework Application has been updated to default the @user@ and
@log@ aliases to the <engine-root>/user and <engine-root>/user/log
folder respectively if a project isn't set.

Fixed the SystemFile class to support negative offsets if Seek() as per
standard seek function such as fseek

Updated the CrySystem CLog class to use SystemFile instead of FileIOBase
to avoid any asserts that would cause CLog::OpenFile to be recursively
called infinitely

* Removing unused Force Closed variable

* AZ::IO::SystemFile build fixes for Unix platforms. Added a copy constructor for LUAEditorContextInterface.h to fix the LuaEditor build

* Adding missing includes to the WindowsAPI and Android SystemFile headers
This commit is contained in:
lumberyard-employee-dm
2021-06-03 22:36:34 -05:00
committed by GitHub
parent 30eedc1c55
commit 4a1d713227
15 changed files with 200 additions and 115 deletions
+51 -55
View File
@@ -26,6 +26,7 @@
#include <AzFramework/IO/FileOperations.h>
#include <AzCore/IO/FileIO.h>
#include <AzCore/IO/Path/Path.h>
#ifdef WIN32
#include <time.h>
@@ -88,7 +89,6 @@ CLog::CLog(ISystem* pSystem)
m_nMainThreadId = CryGetCurrentThreadId();
m_logFileHandle = AZ::IO::InvalidHandle;
#if defined(KEEP_LOG_FILE_OPEN)
m_bFirstLine = true;
#endif
@@ -162,35 +162,6 @@ void CLog::RegisterConsoleVariables()
REGISTER_COMMAND("log_flush", &LogFlushFile, 0, "Flush the log file");
#endif
}
/*
//testbed
{
int iSave0 = m_pLogVerbosity->GetIVal();
int iSave1 = m_pLogFileVerbosity->GetIVal();
for(int i=0;i<=4;++i)
{
m_pLogVerbosity->Set(i);
m_pLogFileVerbosity->Set(i);
LogWithType(eAlways,"CLog selftest: Verbosity=%d FileVerbosity=%d",m_pLogVerbosity->GetIVal(),m_pLogFileVerbosity->GetIVal());
LogWithType(eAlways,"--------------");
LogWithType(eError,"eError");
LogWithType(eWarning,"eWarning");
LogWithType(eMessage,"eMessage");
LogWithType(eInput,"eInput");
LogWithType(eInputResponse,"eInputResponse");
LogWarning("LogWarning()");
LogError("LogError()");
LogWithType(eAlways,"--------------");
}
m_pLogVerbosity->Set(iSave0);
m_pLogFileVerbosity->Set(iSave1);
}
*/
#undef DEFAULT_VERBOSITY
}
@@ -210,7 +181,7 @@ CLog::~CLog()
UnregisterConsoleVariables();
CloseLogFile(true);
CloseLogFile();
}
void CLog::UnregisterConsoleVariables()
@@ -224,31 +195,36 @@ void CLog::UnregisterConsoleVariables()
}
//////////////////////////////////////////////////////////////////////////
void CLog::CloseLogFile([[maybe_unused]] bool forceClose)
void CLog::CloseLogFile()
{
if (m_logFileHandle != AZ::IO::InvalidHandle)
{
AZ::IO::FileIOBase::GetDirectInstance()->Close(m_logFileHandle);
m_logFileHandle = AZ::IO::InvalidHandle;
}
m_logFileHandle.Close();
}
//////////////////////////////////////////////////////////////////////////
AZ::IO::HandleType CLog::OpenLogFile(const char* filename, const char* mode)
bool CLog::OpenLogFile(const char* filename, int mode)
{
using namespace AZ::IO;
AZ_Assert(m_logFileHandle == AZ::IO::InvalidHandle, "Attempt to open log file when one is already open. This would lead to a handle leak.");
if ((!filename) || (filename[0] == 0))
if (m_logFileHandle.IsOpen())
{
return m_logFileHandle;
// Can only AZ_Assert if a file is open, otherwise the AZ_Assert
// would eventually lead to OpenLogFile being opened up again
AZ_Assert(false, "Attempt to open log file when one is already open. This would lead to a handle leak.");
return false;
}
if (filename == nullptr || filename[0] == '\0')
{
return false;
}
// it is assumed that @log@ points at the appropriate place (so for apple, to the user profile dir)
AZ::IO::FileIOBase::GetDirectInstance()->Open(filename, AZ::IO::GetOpenModeFromStringMode(mode), m_logFileHandle);
AZ::IO::FileIOBase* fileSystem = AZ::IO::FileIOBase::GetDirectInstance();
if (AZ::IO::FixedMaxPath logFilePath; fileSystem->ReplaceAlias(logFilePath, filename))
{
logFilePath = logFilePath.LexicallyNormal();
m_logFileHandle.Open(logFilePath.c_str(), mode);
}
if (m_logFileHandle != AZ::IO::InvalidHandle)
if (m_logFileHandle.IsOpen())
{
#if defined(KEEP_LOG_FILE_OPEN)
m_bFirstLine = true;
@@ -257,11 +233,11 @@ AZ::IO::HandleType CLog::OpenLogFile(const char* filename, const char* mode)
else
{
#if defined(LINUX) || defined(APPLE)
syslog(LOG_NOTICE, "Failed to open log file [%s], mode [%s]", filename, mode);
syslog(LOG_NOTICE, "Failed to open log file [%s], mode [%d]", filename, mode);
#endif
}
return m_logFileHandle;
return m_logFileHandle.IsOpen();
}
//////////////////////////////////////////////////////////////////////////
@@ -1114,12 +1090,15 @@ void CLog::LogStringToFile(const char* szString, ELogType logType, bool bAdd, [[
if (logToFile)
{
if (m_logFileHandle == AZ::IO::InvalidHandle)
if (!m_logFileHandle.IsOpen())
{
OpenLogFile(m_szFilename, "w+t");
constexpr auto openMode = AZ::IO::SystemFile::OpenMode::SF_OPEN_APPEND
| AZ::IO::SystemFile::OpenMode::SF_OPEN_CREATE
| AZ::IO::SystemFile::OpenMode::SF_OPEN_WRITE_ONLY;
OpenLogFile(m_szFilename, openMode);
}
if (m_logFileHandle != AZ::IO::InvalidHandle)
if (m_logFileHandle.IsOpen())
{
#if defined(KEEP_LOG_FILE_OPEN)
if (m_bFirstLine)
@@ -1130,9 +1109,9 @@ void CLog::LogStringToFile(const char* szString, ELogType logType, bool bAdd, [[
if (bAdd)
{
// if adding to a prior line erase the \n at the end.
AZ::IO::FileIOBase::GetDirectInstance()->Seek(m_logFileHandle, -2, AZ::IO::SeekType::SeekFromEnd);
m_logFileHandle.Seek(-2, AZ::IO::SystemFile::SeekMode::SF_SEEK_END);
}
AZ::IO::FPutS(tempString.c_str(), m_logFileHandle);
m_logFileHandle.Write(tempString.c_str(), tempString.size());
#if !defined(KEEP_LOG_FILE_OPEN)
CloseLogFile();
#endif
@@ -1383,6 +1362,23 @@ bool CLog::SetFileName(const char* fileNameOrAbsolutePath, bool backupLogs)
CreateBackupFile();
AZ::IO::FileIOBase* fileSystem = AZ::IO::FileIOBase::GetDirectInstance();
AZ::IO::FixedMaxPath newLogFilePath;
if (fileSystem->ReplaceAlias(newLogFilePath, m_szFilename))
{
newLogFilePath = newLogFilePath.LexicallyNormal();
}
if (m_logFileHandle.IsOpen() && newLogFilePath != m_logFileHandle.Name())
{
constexpr auto openMode = AZ::IO::SystemFile::OpenMode::SF_OPEN_APPEND
| AZ::IO::SystemFile::OpenMode::SF_OPEN_CREATE
| AZ::IO::SystemFile::OpenMode::SF_OPEN_WRITE_ONLY;
if(AZ::IO::SystemFile newLogFile; newLogFile.Open(m_szFilename, openMode))
{
m_logFileHandle = AZStd::move(newLogFile);
}
}
return true;
}
@@ -1537,9 +1533,9 @@ const char* CLog::GetModuleFilter()
void CLog::FlushAndClose()
{
#if defined(KEEP_LOG_FILE_OPEN)
if (m_logFileHandle)
if (m_logFileHandle.IsOpen())
{
CloseLogFile(true);
CloseLogFile();
}
#endif
}
+7 -11
View File
@@ -137,8 +137,8 @@ private: // -------------------------------------------------------------------
void LogStringToConsole(const char* szString, ELogType logType, bool bAdd) {}
#endif // !defined(EXCLUDE_NORMAL_LOG)
AZ::IO::HandleType OpenLogFile(const char* filename, const char* mode);
void CloseLogFile(bool force = false);
bool OpenLogFile(const char* filename, int mode);
void CloseLogFile();
// will format the message into m_szTemp
void FormatMessage(const char* szCommand, ...) PRINTF_PARAMS(2, 3);
@@ -152,15 +152,11 @@ private: // -------------------------------------------------------------------
virtual const char* GetAssetScopeString();
#endif
ISystem* m_pSystem; //
float m_fLastLoadingUpdateTime; // for non-frequent streamingEngine update
//char m_szTemp[MAX_TEMP_LENGTH_SIZE]; //
char m_szFilename[MAX_FILENAME_SIZE]; // can be with path
mutable char m_sBackupFilename[MAX_FILENAME_SIZE]; // can be with path
AZ::IO::HandleType m_logFileHandle;
CryStackStringT<char, 32> m_LogMode; //mode m_pLogFile has been opened with
AZ::IO::HandleType m_errFileHandle;
int m_nErrCount;
ISystem* m_pSystem; //
float m_fLastLoadingUpdateTime; // for non-frequent streamingEngine update
char m_szFilename[MAX_FILENAME_SIZE]; // can be with path
mutable char m_sBackupFilename[MAX_FILENAME_SIZE]; // can be with path
AZ::IO::SystemFile m_logFileHandle;
bool m_backupLogs;
+1 -1
View File
@@ -1208,7 +1208,7 @@ bool CSystem::Init(const SSystemInitParams& startupParams)
{
assetPlatform = AzFramework::OSPlatformToDefaultAssetPlatform(AZ_TRAIT_OS_PLATFORM_CODENAME);
AZ_Warning(AZ_TRACE_SYSTEM_WINDOW, false, R"(A valid asset platform is missing in "%s/assets" key in the SettingsRegistry.)""\n"
R"(This typically done by setting he "assets" field in the bootstrap.cfg for within a .setreg file)""\n"
R"(This typically done by setting the "assets" field within a .setreg file)""\n"
R"(A fallback of %s will be used.)",
AZ::SettingsRegistryMergeUtils::BootstrapSettingsRootKey,
assetPlatform.c_str());