From 686048dcce5bc90beee962c394a857342c0ec674 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Fri, 21 Jul 2023 15:40:19 -0700 Subject: [PATCH] Be smarter about releasing lockfiles If KiCad crashes or exits without deleting the lockfile, don't show the warning message unless we are not the one who locked it or there are other KiCad instances running locally. This should catch 99% of the cases where the message is shown incorrectly. There may be some corner cases where the lock file is created on a network drive using two different machines with the same name and same user but these cases should be (famous last words) sufficiently rare as to not be observed in practice (cherry picked from commit 7fe83993cfc423531dbb96962f48f8601ea1d84e) --- common/confirm.cpp | 2 +- common/eda_draw_frame.cpp | 9 +++++++++ common/pgm_base.cpp | 4 ++++ eeschema/files-io.cpp | 2 +- include/confirm.h | 2 +- include/lockfile.h | 5 +++++ include/pgm_base.h | 14 +++++++++++++- pcbnew/files.cpp | 16 ++++++++++++++-- 8 files changed, 48 insertions(+), 6 deletions(-) diff --git a/common/confirm.cpp b/common/confirm.cpp index 6020f619a9..084611615c 100644 --- a/common/confirm.cpp +++ b/common/confirm.cpp @@ -155,7 +155,7 @@ long KIDIALOG::getStyle( KD_TYPE aType ) } -bool OverrideLock( wxWindow* aParent, const wxString& aMessage ) +bool AskOverrideLock( wxWindow* aParent, const wxString& aMessage ) { #ifdef __APPLE__ // wxMessageDialog gets the button spacing wrong on Mac so we have to use wxRichMessageDialog. diff --git a/common/eda_draw_frame.cpp b/common/eda_draw_frame.cpp index f2923b5405..57665d58b9 100644 --- a/common/eda_draw_frame.cpp +++ b/common/eda_draw_frame.cpp @@ -230,6 +230,15 @@ bool EDA_DRAW_FRAME::LockFile( const wxString& aFileName ) m_file_checker = std::make_unique( aFileName ); + if( !m_file_checker->Valid() && m_file_checker->IsLockedByMe() ) + { + // If we cannot acquire the lock but we appear to be the one who + // locked it, check to see if there is another KiCad instance running. + // If there is not, then we can override the lock. This could happen if + // KiCad crashed or was interrupted + if( !Pgm().SingleInstance()->IsAnotherRunning() ) + m_file_checker->OverrideLock(); + } // If the file is valid, return true. This could mean that the file is // locked or it could mean that the file is read-only return m_file_checker->Valid(); diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp index 3abb32a0f7..8899927df2 100644 --- a/common/pgm_base.cpp +++ b/common/pgm_base.cpp @@ -161,6 +161,8 @@ void PGM_BASE::Destroy() // unlike a normal destructor, this is designed to be called more than once safely: delete m_locale; m_locale = nullptr; + + m_pgm_checker.reset(); } @@ -443,6 +445,8 @@ bool PGM_BASE::InitPgm( bool aHeadless, bool aSkipPyInit, bool aIsUnitTest ) return false; } #endif + m_pgm_checker = std::make_unique(); + m_pgm_checker->Create( pgm_name, wxStandardPaths::Get().GetTempDir() ); // Init KiCad environment // the environment variable KICAD (if exists) gives the kicad path: diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index 7ac61f4ed7..4d6217444b 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -103,7 +103,7 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in { msg.Printf( _( "Schematic '%s' is already open." ), wx_filename.GetFullName() ); - if( !OverrideLock( this, msg ) ) + if( !AskOverrideLock( this, msg ) ) return false; m_file_checker->OverrideLock( false ); diff --git a/include/confirm.h b/include/confirm.h index 5fe5d75c70..d7967af2ec 100644 --- a/include/confirm.h +++ b/include/confirm.h @@ -86,7 +86,7 @@ protected: * Display a dialog indicating the file is already open, with an option to reset the lock. * @return true if the lock was reset. */ -bool OverrideLock( wxWindow* aParent, const wxString& aMessage ); +bool AskOverrideLock( wxWindow* aParent, const wxString& aMessage ); /** diff --git a/include/lockfile.h b/include/lockfile.h index a3c52a4dae..3b1a3e9170 100644 --- a/include/lockfile.h +++ b/include/lockfile.h @@ -207,6 +207,11 @@ public: return true; } + bool IsLockedByMe() + { + return m_username == wxGetUserId() && m_hostname == wxGetHostName(); + } + /** * @return Current username. If we own the lock, this is us. Otherwise, this is the user that does own it */ diff --git a/include/pgm_base.h b/include/pgm_base.h index f02b493447..83e2a54da1 100644 --- a/include/pgm_base.h +++ b/include/pgm_base.h @@ -38,11 +38,11 @@ #include #include #include +#include #undef pid_t #include -class wxSingleInstanceChecker; class wxApp; class wxMenu; class wxWindow; @@ -313,6 +313,14 @@ public: */ bool IsGUI(); + /** + * Allows access to the wxSingleInstanceChecker to test for other running KiCads + */ + std::unique_ptr& SingleInstance() + { + return m_pgm_checker; + } + /** * wxWidgets on MSW tends to crash if you spool up more than one print job at a time. */ @@ -347,6 +355,10 @@ protected: std::unique_ptr m_python_scripting; + /// Checks if there is another copy of Kicad running at the same time + std::unique_ptr m_pgm_checker; + + wxString m_bin_dir; /// full path to this program wxString m_kicad_env; /// The KICAD system environment variable. diff --git a/pcbnew/files.cpp b/pcbnew/files.cpp index 5a9e812b75..80ba36ec2d 100644 --- a/pcbnew/files.cpp +++ b/pcbnew/files.cpp @@ -53,6 +53,7 @@ #include #include #include +#include #include #include #include @@ -611,12 +612,23 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in std::unique_ptr lock = std::make_unique( fullFileName ); - if( !lock->Locked() ) + if( !lock->Valid() && lock->IsLockedByMe() ) + { + // If we cannot acquire the lock but we appear to be the one who + // locked it, check to see if there is another KiCad instance running. + // If there is not, then we can override the lock. This could happen if + // KiCad crashed or was interrupted + + if( !Pgm().SingleInstance()->IsAnotherRunning() ) + lock->OverrideLock(); + } + + if( !lock->Valid() ) { msg.Printf( _( "PCB '%s' is already open by '%s' at '%s'." ), wx_filename.GetFullName(), lock->GetUsername(), lock->GetHostname() ); - if( !OverrideLock( this, msg ) ) + if( !AskOverrideLock( this, msg ) ) return false; lock->OverrideLock();