From 21595f48d38a15e22f60c51f96e7a82b6a6e1188 Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Fri, 10 Jun 2016 13:47:19 -0400 Subject: [PATCH] KiCad: fix crash when kiface libraries are missing. (fixes lp:1577786) * An assumption was made that wxDynamicLibrary.Load() would always result in a wxLogSystemError on failure which was not always true. The code now throws an exception which is caught by KiCad and an error message is displayed. In the case where the wxLogSystemError is shown, there will be an annoying two error messages but that is better than a crash. --- common/kiway.cpp | 36 ++++++++++++++++++++++------------ kicad/mainframe.cpp | 48 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 67 insertions(+), 17 deletions(-) diff --git a/common/kiway.cpp b/common/kiway.cpp index d7a870fc03..dcfdaa14e3 100644 --- a/common/kiway.cpp +++ b/common/kiway.cpp @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2014 SoftPLC Corporation, Dick Hollenbeck - * Copyright (C) 2014 KiCad Developers, see CHANGELOG.TXT for contributors. + * Copyright (C) 2014-2016 KiCad Developers, see CHANGELOG.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 @@ -62,7 +62,7 @@ KIWAY::KIWAY( PGM_BASE* aProgram, int aCtlBits, wxFrame* aTop ): #if 0 // Any event types derived from wxCommandEvt, like wxWindowDestroyEvent, are -// propogated upwards to parent windows if not handled below. Therefore the +// propagated upwards to parent windows if not handled below. Therefore the // m_top window should receive all wxWindowDestroyEvents originating from // KIWAY_PLAYERs. It does anyways, but now player_destroy_handler eavesdrops // on that event stream looking for KIWAY_PLAYERs being closed. @@ -74,6 +74,7 @@ void KIWAY::player_destroy_handler( wxWindowDestroyEvent& event ) } #endif + void KIWAY::SetTop( wxFrame* aTop ) { #if 0 @@ -153,6 +154,8 @@ KIFACE* KIWAY::KiFACE( FACE_T aFaceId, bool doLoad ) if( m_kiface[aFaceId] ) return m_kiface[aFaceId]; + wxString msg; + // DSO with KIFACE has not been loaded yet, does caller want to load it? if( doLoad ) { @@ -165,15 +168,24 @@ KIFACE* KIWAY::KiFACE( FACE_T aFaceId, bool doLoad ) if( !dso.Load( dname, wxDL_VERBATIM | wxDL_NOW | wxDL_GLOBAL ) ) { // Failure: error reporting UI was done via wxLogSysError(). - // No further reporting required here. - } + // No further reporting required here. Apparently this is not true on all + // platforms and/or wxWidgets builds and KiCad will crash. Throwing the exception + // here and catching it in the KiCad launcher resolves the crash issue. See bug + // report https://bugs.launchpad.net/kicad/+bug/1577786. + msg.Printf( _( "Failed to load kiface library '%s'." ), GetChars( dname ) ); + THROW_IO_ERROR( msg ); + } else if( ( addr = dso.GetSymbol( wxT( KIFACE_INSTANCE_NAME_AND_VERSION ) ) ) == NULL ) { // Failure: error reporting UI was done via wxLogSysError(). - // No further reporting required here. + // No further reporting required here. Assume the same thing applies here as + // above with the Load() call. This has not been tested. + msg.Printf( + _( "Could not read instance name and version symbol form kiface library '%s'." ), + GetChars( dname ) ); + THROW_IO_ERROR( msg ); } - else { KIFACE_GETTER_FUNC* getter = (KIFACE_GETTER_FUNC*) addr; @@ -182,7 +194,7 @@ KIFACE* KIWAY::KiFACE( FACE_T aFaceId, bool doLoad ) // KIFACE_GETTER_FUNC function comment (API) says the non-NULL is unconditional. wxASSERT_MSG( kiface, - wxT( "attempted DSO has a bug, failed to return a KIFACE*" ) ); + wxT( "attempted DSO has a bug, failed to return a KIFACE*" ) ); // Give the DSO a single chance to do its "process level" initialization. // "Process level" specifically means stay away from any projects in there. @@ -203,16 +215,16 @@ KIFACE* KIWAY::KiFACE( FACE_T aFaceId, bool doLoad ) // to exist, and we did not find one. If we do not find one, this is an // installation bug. - wxString msg = wxString::Format( wxT( + msg = wxString::Format( _( "Fatal Installation Bug. File:\n" "'%s'\ncould not be loaded\n" ), GetChars( dname ) ); if( ! wxFileExists( dname ) ) - msg << wxT( "It is missing.\n" ); + msg << _( "It is missing.\n" ); else - msg << wxT( "Perhaps a shared library (.dll or .so) file is missing.\n" ); + msg << _( "Perhaps a shared library (.dll or .so) file is missing.\n" ); - msg << wxT( "From command line: argv[0]:\n'" ); + msg << _( "From command line: argv[0]:\n'" ); msg << wxStandardPaths::Get().GetExecutablePath() << wxT( "'\n" ); // This is a fatal error, one from which we cannot recover, nor do we want @@ -276,7 +288,6 @@ KIWAY_PLAYER* KIWAY::GetPlayerFrame( FRAME_T aFrameType ) } - KIWAY_PLAYER* KIWAY::Player( FRAME_T aFrameType, bool doCreate, KIWAY_PLAYER* aParent ) { // Since this will be called from python, cannot assume that code will @@ -442,4 +453,3 @@ void KIWAY::OnKiwayEnd() m_kiface[i]->OnKifaceEnd(); } } - diff --git a/kicad/mainframe.cpp b/kicad/mainframe.cpp index a1d20dae34..6f4af07ee1 100644 --- a/kicad/mainframe.cpp +++ b/kicad/mainframe.cpp @@ -299,7 +299,16 @@ void KICAD_MANAGER_FRAME::RunEeschema( const wxString& aProjectSchematicFileName // and the dialog field editor was used if( !frame ) { - frame = Kiway.Player( FRAME_SCH, true ); + try + { + frame = Kiway.Player( FRAME_SCH, true ); + } + catch( IO_ERROR err ) + { + wxMessageBox( _( "Eeschema failed to load:\n" ) + err.errorText, + _( "KiCad Error" ), wxOK | wxICON_ERROR, this ); + return; + } } if( !frame->IsShown() ) // the frame exists, (created by the dialog field editor) @@ -334,7 +343,17 @@ void KICAD_MANAGER_FRAME::OnRunSchLibEditor( wxCommandEvent& event ) if( !frame ) { - frame = Kiway.Player( FRAME_SCH_LIB_EDITOR, true ); + try + { + frame = Kiway.Player( FRAME_SCH_LIB_EDITOR, true ); + } + catch( IO_ERROR err ) + { + wxMessageBox( _( "Component library editor failed to load:\n" ) + err.errorText, + _( "KiCad Error" ), wxOK | wxICON_ERROR, this ); + return; + } + // frame->OpenProjectFiles( std::vector( 1, aProjectSchematicFileName ) ); frame->Show( true ); } @@ -349,7 +368,18 @@ void KICAD_MANAGER_FRAME::OnRunSchLibEditor( wxCommandEvent& event ) void KICAD_MANAGER_FRAME::RunPcbNew( const wxString& aProjectBoardFileName ) { - KIWAY_PLAYER* frame = Kiway.Player( FRAME_PCB, true ); + KIWAY_PLAYER* frame; + + try + { + frame = Kiway.Player( FRAME_PCB, true ); + } + catch( IO_ERROR err ) + { + wxMessageBox( _( "Pcbnew failed to load:\n" ) + err.errorText, _( "KiCad Error" ), + wxOK | wxICON_ERROR, this ); + return; + } // a pcb frame can be already existing, but not yet used. // this is the case when running the footprint editor, or the footprint viewer first @@ -386,7 +416,17 @@ void KICAD_MANAGER_FRAME::OnRunPcbFpEditor( wxCommandEvent& event ) if( !frame ) { - frame = Kiway.Player( FRAME_PCB_MODULE_EDITOR, true ); + try + { + frame = Kiway.Player( FRAME_PCB_MODULE_EDITOR, true ); + } + catch( IO_ERROR err ) + { + wxMessageBox( _( "Footprint library editor failed to load:\n" ) + err.errorText, + _( "KiCad Error" ), wxOK | wxICON_ERROR, this ); + return; + } + // frame->OpenProjectFiles( std::vector( 1, aProjectBoardFileName ) ); frame->Show( true ); }