From 6fc393c7db1d5294f49713a0951b2a87f4c1874f Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Wed, 24 May 2023 13:19:48 -0700 Subject: [PATCH] Upgrade file locking wxSingleInstanceChecker is meant for running programs, not file locking. This implements an RAII class for file locking that stores the lock files next to the file being locked, allowing it to be easily found and removed. Also includes the ability to override the lock, with information about the original owner Fixes https://gitlab.com/kicad/code/kicad/-/issues/14734 (cherry picked from commit 122be418bb69301b118577a42a2efbc198622704) --- common/CMakeLists.txt | 1 - common/eda_draw_frame.cpp | 11 +- common/lockfile.cpp | 96 ---------- common/pgm_base.cpp | 3 +- common/settings/settings_manager.cpp | 4 +- eeschema/files-io.cpp | 8 +- include/eda_draw_frame.h | 3 +- include/lockfile.h | 261 +++++++++++++++++++++++++-- include/settings/settings_manager.h | 3 +- pcbnew/files.cpp | 14 +- 10 files changed, 277 insertions(+), 127 deletions(-) delete mode 100644 common/lockfile.cpp diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index c7f3150bac..6934411ddd 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -353,7 +353,6 @@ set( COMMON_SRCS lib_tree_model.cpp lib_tree_model_adapter.cpp locale_io.cpp - lockfile.cpp lset.cpp marker_base.cpp markup_parser.cpp diff --git a/common/eda_draw_frame.cpp b/common/eda_draw_frame.cpp index 4b50a7b1d6..10ec761b35 100644 --- a/common/eda_draw_frame.cpp +++ b/common/eda_draw_frame.cpp @@ -217,15 +217,20 @@ EDA_DRAW_FRAME::~EDA_DRAW_FRAME() void EDA_DRAW_FRAME::ReleaseFile() { - m_file_checker = nullptr; + if( m_file_checker.get() != nullptr ) + m_file_checker->UnlockFile(); } bool EDA_DRAW_FRAME::LockFile( const wxString& aFileName ) { - m_file_checker = ::LockFile( aFileName ); + // We need to explicitly reset here to get the deletion before + // we create a new unique_ptr that may be for the same file + m_file_checker.reset(); - return m_file_checker && !m_file_checker->IsAnotherRunning(); + m_file_checker = std::make_unique( aFileName ); + + return m_file_checker->Locked(); } diff --git a/common/lockfile.cpp b/common/lockfile.cpp deleted file mode 100644 index 889e821318..0000000000 --- a/common/lockfile.cpp +++ /dev/null @@ -1,96 +0,0 @@ -/* - * This program source code file is part of KiCad, a free EDA CAD application. - * - * Copyright (C) 2014-2015 SoftPLC Corporation, Dick Hollenbeck - * Copyright (C) 2014-2022 KiCad Developers, see AUTHORS.TXT for contributors. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 2 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, you may find one here: - * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html - * or you may search the http://www.gnu.org website for the version 2 license, - * or you may write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA - */ - -#include - -#include -#include - -#include -#include - - -std::unique_ptr LockFile( const wxString& aFileName ) -{ - // first make absolute and normalize, to avoid that different lock files - // for the same file can be created - wxFileName fn( aFileName ); - - fn.MakeAbsolute(); - - wxString lockFileName = fn.GetFullPath() + ".lock"; - - lockFileName.Replace( "/", "_" ); - - // We can have filenames coming from Windows, so also convert Windows separator - lockFileName.Replace( "\\", "_" ); - - auto p = std::make_unique( lockFileName, GetKicadLockFilePath() ); - - if( p->IsAnotherRunning() ) - p = nullptr; - - return p; -} - - -wxString GetKicadLockFilePath() -{ - wxFileName lockpath; - lockpath.AssignDir( wxGetHomeDir() ); // Default wx behavior - -#if defined( __WXMAC__ ) - // In OSX use the standard per user cache directory - lockpath.AppendDir( "Library" ); - lockpath.AppendDir( "Caches" ); - lockpath.AppendDir( "kicad" ); -#elif defined( __UNIX__ ) - wxString envstr; - // Try first the standard XDG_RUNTIME_DIR, falling back to XDG_CACHE_HOME - if( wxGetEnv( "XDG_RUNTIME_DIR", &envstr ) && !envstr.IsEmpty() ) - { - lockpath.AssignDir( envstr ); - } - else if( wxGetEnv( "XDG_CACHE_HOME", &envstr ) && !envstr.IsEmpty() ) - { - lockpath.AssignDir( envstr ); - } - else - { - // If all fails, just use ~/.cache - lockpath.AppendDir( ".cache" ); - } - - lockpath.AppendDir( wxString::Format( "kicad_v%s", GetMajorMinorVersion() ) ); -#endif - -#if defined( __WXMAC__ ) || defined( __UNIX__ ) - if( !lockpath.DirExists() ) - { - // Lockfiles should be only readable by the user - lockpath.Mkdir( 0700, wxPATH_MKDIR_FULL ); - } -#endif - return lockpath.GetPath(); -} diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp index 4ed94eea75..2caf370774 100644 --- a/common/pgm_base.cpp +++ b/common/pgm_base.cpp @@ -54,7 +54,6 @@ #include #include #include -#include #include #include #include @@ -913,4 +912,4 @@ void PGM_BASE::HandleException( std::exception_ptr aPtr ) { wxLogError( wxT( "Unhandled exception of unknown type" ) ); } -} \ No newline at end of file +} diff --git a/common/settings/settings_manager.cpp b/common/settings/settings_manager.cpp index 0d45ba23fd..389a4ab28f 100644 --- a/common/settings/settings_manager.cpp +++ b/common/settings/settings_manager.cpp @@ -892,7 +892,7 @@ bool SETTINGS_MANAGER::LoadProject( const wxString& aFullPath, bool aSetActive ) return true; bool readOnly = false; - std::unique_ptr lockFile = ::LockFile( fullPath ); + LOCKFILE lockFile( fullPath ); if( !lockFile ) { @@ -929,7 +929,7 @@ bool SETTINGS_MANAGER::LoadProject( const wxString& aFullPath, bool aSetActive ) project->SetReadOnly( readOnly || project->GetProjectFile().IsReadOnly() ); if( lockFile ) - m_project_lock.reset( lockFile.release() ); + m_project_lock.reset( new LOCKFILE( std::move( lockFile ) ) ); } m_projects_list.push_back( std::move( project ) ); diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index d846abc1ea..fcc3adf229 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -104,6 +105,8 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in if( !OverrideLock( this, msg ) ) return false; + + m_file_checker->OverrideLock( false ); } if( !AskToSaveChanges() ) @@ -1214,10 +1217,13 @@ bool SCH_EDIT_FRAME::importFile( const wxString& aFileName, int aFileType ) if( !LockFile( aFileName ) ) { wxString msg; - msg.Printf( _( "Schematic '%s' is already open." ), filename.GetFullName() ); + msg.Printf( _( "Schematic '%s' is already open by '%s' at '%s'." ), aFileName, + m_file_checker->GetUsername(), m_file_checker->GetHostname() ); if( !OverrideLock( this, msg ) ) return false; + + m_file_checker->OverrideLock(); } try diff --git a/include/eda_draw_frame.h b/include/eda_draw_frame.h index 3e5b0a2943..b884179fc1 100644 --- a/include/eda_draw_frame.h +++ b/include/eda_draw_frame.h @@ -38,6 +38,7 @@ class EDA_ITEM; class wxSingleInstanceChecker; class ACTION_TOOLBAR; class COLOR_SETTINGS; +class LOCKFILE; class TOOL_MENU; class APP_SETTINGS_BASE; class wxFindReplaceData; @@ -490,7 +491,7 @@ protected: std::vector m_sockets; ///< interprocess communication ///< Prevents opening same file multiple times. - std::unique_ptr m_file_checker; + std::unique_ptr m_file_checker; COLOR4D m_gridColor; // Grid color COLOR4D m_drawBgColor; // The background color of the draw canvas; BLACK for diff --git a/include/lockfile.h b/include/lockfile.h index 3c31904d51..99a8328b67 100644 --- a/include/lockfile.h +++ b/include/lockfile.h @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2017-2020 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -29,20 +29,255 @@ #ifndef INCLUDE__LOCK_FILE_H_ #define INCLUDE__LOCK_FILE_H_ -#include -#include +#include +#include +#include +#include +#include +#include -class wxSingleInstanceChecker; +#define LCK "KICAD_LOCKING" -/** - * Test to see if \a aFileName can be locked (is not already locked) and only then - * returns a wxSingleInstanceChecker protecting aFileName. - */ -std::unique_ptr LockFile( const wxString& aFileName ); +class LOCKFILE +{ +public: + LOCKFILE( const wxString &filename, bool aRemoveOnRelease = true ) : + m_originalFile( filename ), m_fileCreated( false ), m_status( false ), + m_removeOnRelease( aRemoveOnRelease ), m_errorMsg( "" ) + { + if( filename.IsEmpty() ) + return; + + wxFileName fn( filename ); + fn.SetName( "~" + fn.GetName() ); + fn.SetExt( fn.GetExt() + ".lck" ); + + m_lockFilename = fn.GetFullPath(); + + wxFile file; + try + { + bool lock_success = false; + bool rw_success = false; + + { + wxLogNull suppressExpectedErrorMessages; + + lock_success = file.Open( m_lockFilename, wxFile::write_excl ); + + if( !lock_success ) + rw_success = file.Open( m_lockFilename, wxFile::read ); + } + + if( lock_success ) + { + // Lock file doesn't exist, create one + m_fileCreated = true; + m_status = true; + m_username = wxGetUserId(); + m_hostname = wxGetHostName(); + nlohmann::json j; + j["username"] = std::string( m_username.mb_str() ); + j["hostname"] = std::string( m_hostname.mb_str() ); + std::string lock_info = j.dump(); + file.Write( lock_info ); + file.Close(); + wxLogTrace( LCK, "Locked %s", filename ); + } + else if( rw_success ) + { + // Lock file already exists, read the details + wxString lock_info; + file.ReadAll( &lock_info ); + nlohmann::json j = nlohmann::json::parse( std::string( lock_info.mb_str() ) ); + m_username = wxString( j["username"].get() ); + m_hostname = wxString( j["hostname"].get() ); + file.Close(); + m_errorMsg = _( "Lock file already exists" ); + wxLogTrace( LCK, "Existing Lock for %s", filename ); + } + else + { + throw; + } + } + catch( std::exception& e ) + { + wxLogError( "Got an error trying to lock %s: %s", filename, e.what() ); + + // Delete lock file if it was created above but we threw an exception somehow + if( m_fileCreated ) + { + wxRemoveFile( m_lockFilename ); + m_fileCreated = false; // Reset the flag since file has been deleted manually + } + + m_errorMsg = _( "Failed to access lock file" ); + m_status = false; + } + } + + ~LOCKFILE() + { + UnlockFile(); + } + + /** + * Unlocks and removes the file from the filesystem as long as we still own it. + */ + void UnlockFile() + { + wxLogTrace( LCK, "Unlocking %s", m_lockFilename ); + + // Delete lock file only if the file was created in the constructor and if the file contains the correct user and host names + if( m_fileCreated && checkUserAndHost() ) + { + if( m_removeOnRelease ) + wxRemoveFile( m_lockFilename ); + + m_fileCreated = false; // Reset the flag since file has been deleted manually + m_status = false; + m_errorMsg = wxEmptyString; + } + } + + /** + * Forces the lock, overwriting the data that existed already + * @return True if we successfully overrode the lock + */ + bool OverrideLock( bool aRemoveOnRelease = true ) + { + wxLogTrace( LCK, "Overriding lock on %s", m_lockFilename ); + + if( !m_fileCreated ) + { + try + { + wxFile file; + bool success = false; + + { + wxLogNull suppressExpectedErrorMessages; + success = file.Open( m_lockFilename, wxFile::write ); + } + + if( success ) + { + m_username = wxGetUserId(); + m_hostname = wxGetHostName(); + nlohmann::json j; + j["username"] = std::string( m_username.mb_str() ); + j["hostname"] = std::string( m_hostname.mb_str() ); + std::string lock_info = j.dump(); + file.Write( lock_info ); + file.Close(); + m_fileCreated = true; + m_status = true; + m_removeOnRelease = aRemoveOnRelease; + m_errorMsg = wxEmptyString; + wxLogTrace( LCK, "Successfully overrode lock on %s", m_lockFilename ); + return true; + } + + return false; + } + catch( std::exception& e ) + { + wxLogError( "Got exception trying to override lock on %s: %s", + m_lockFilename, e.what() ); + + return false; + } + } + else + { + wxLogTrace( LCK, "Upgraded lock on %s to delete on release", m_lockFilename ); + m_removeOnRelease = aRemoveOnRelease; + } + + return true; + } + + /** + * @return Current username. If we own the lock, this is us. Otherwise, this is the user that does own it + */ + wxString GetUsername(){ return m_username; } + + /** + * @return Current hostname. If we own the lock this is our computer. Otherwise, this is the computer that does + */ + wxString GetHostname(){ return m_hostname; } + + /** + * @return Last error message generated + */ + wxString GetErrorMsg(){ return m_errorMsg; } + + bool Locked() const + { + return m_fileCreated; + } + + bool Valid() const + { + return m_status; + } + + explicit operator bool() const + { + return m_status; + } + +private: + wxString m_originalFile; + wxString m_lockFilename; + wxString m_username; + wxString m_hostname; + bool m_fileCreated; + bool m_status; + bool m_removeOnRelease; + wxString m_errorMsg; + + bool checkUserAndHost() + { + wxFileName fileName( m_lockFilename ); + + if( !fileName.FileExists() ) + { + wxLogTrace + ( LCK, "File does not exist: %s", m_lockFilename ); + return false; + } + + wxFile file; + try + { + if( file.Open( m_lockFilename, wxFile::read ) ) + { + wxString lock_info; + file.ReadAll( &lock_info ); + nlohmann::json j = nlohmann::json::parse( std::string( lock_info.mb_str() ) ); + if( m_username == wxString( j["username"].get() ) + && m_hostname == wxString( j["hostname"].get() ) ) + { + wxLogTrace + ( LCK, "User and host match for lock %s", m_lockFilename ); + return true; + } + } + } + catch( std::exception &e ) + { + wxLogError + ( "Got exception trying to check user/host for lock on %s: %s", m_lockFilename, + e.what() ); + } + + wxLogTrace + ( LCK, "User and host DID NOT match for lock %s", m_lockFilename ); + return false; + } +}; -/** - * @return A wxString containing the path for lockfiles in Kicad. - */ -wxString GetKicadLockFilePath(); #endif // INCLUDE__LOCK_FILE_H_ diff --git a/include/settings/settings_manager.h b/include/settings/settings_manager.h index 6b98611fdd..8711879f65 100644 --- a/include/settings/settings_manager.h +++ b/include/settings/settings_manager.h @@ -34,6 +34,7 @@ class PROJECT; class PROJECT_FILE; class REPORTER; class wxSingleInstanceChecker; +class LOCKFILE; /// Project settings path will be + this @@ -447,7 +448,7 @@ private: std::map m_project_files; /// Lock for loaded project (expand to multiple once we support MDI) - std::unique_ptr m_project_lock; + std::unique_ptr m_project_lock; static wxString backupDateTimeFormat; }; diff --git a/pcbnew/files.cpp b/pcbnew/files.cpp index 427daffcdd..371f417814 100644 --- a/pcbnew/files.cpp +++ b/pcbnew/files.cpp @@ -609,14 +609,17 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in // We insist on caller sending us an absolute path, if it does not, we say it's a bug. wxASSERT_MSG( wx_filename.IsAbsolute(), wxT( "Path is not absolute!" ) ); - std::unique_ptr lockFile = ::LockFile( fullFileName ); + std::unique_ptr lock = std::make_unique( fullFileName ); - if( !lockFile || lockFile->IsAnotherRunning() ) + if( !lock->Locked() ) { - msg.Printf( _( "PCB '%s' is already open." ), wx_filename.GetFullName() ); + msg.Printf( _( "PCB '%s' is already open by '%s' at '%s'." ), wx_filename.GetFullName(), + lock->GetUsername(), lock->GetHostname() ); if( !OverrideLock( this, msg ) ) return false; + + lock->OverrideLock(); } if( IsContentModified() ) @@ -631,9 +634,6 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in } } - // Release the lock file, until the new file is actually loaded - ReleaseFile(); - wxFileName pro = fullFileName; pro.SetExt( ProjectFileExtension ); @@ -954,7 +954,7 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in } // Lock the file newly opened: - m_file_checker.reset( lockFile.release() ); + m_file_checker.reset( lock.release() ); if( !converted ) UpdateFileHistory( GetBoard()->GetFileName() );