Change OperationDetail to not store complex objects in STL containers (#729139)

OperationDetail was storing its children in a std::vector.  This means they
can be moved around in memory arbitrarily, going through indeterminate
lifetimes.  This is generally a bad thing for any non trivial object and
in the case of OperationDetail, it created havoc with the way it maintains
pointers between parent/child objects for signal connections.  It will now
keep only pointers to children in a std::vector instead, so their lifetime
can be controlled, fixing various crashes.

Bug 729139 - Refactor OperationDetail to address random behavior
This commit is contained in:
Phillip Susi 2014-04-01 22:04:01 -04:00 committed by Curtis Gedak
parent 2e7b5d05a6
commit 947cd02857
5 changed files with 27 additions and 21 deletions

View File

@ -89,7 +89,7 @@ private:
treeview_operations_Columns treeview_operations_columns;
std::vector<Operation *> operations ;
OperationDetail operationdetail ;
Glib::ustring progress_text;
bool succes, cancel;
double fraction ;
unsigned int t, warnings ;

View File

@ -60,8 +60,8 @@ public:
Glib::ustring get_elapsed_time() const ;
void add_child( const OperationDetail & operationdetail ) ;
std::vector<OperationDetail> & get_childs() ;
const std::vector<OperationDetail> & get_childs() const ;
std::vector<OperationDetail*> & get_childs() ;
const std::vector<OperationDetail*> & get_childs() const ;
OperationDetail & get_last_child() ;
double fraction ;
@ -78,7 +78,7 @@ private:
Glib::ustring treepath ;
std::vector<OperationDetail> sub_details ;
std::vector<OperationDetail*> sub_details;
std::time_t time_start, time_elapsed ;
sigc::connection cancelconnection;
};

View File

@ -148,7 +148,7 @@ void Dialog_Progress::on_signal_update( const OperationDetail & operationdetail
}
//update the gui elements..
this ->operationdetail = operationdetail ;
progress_text = operationdetail.progress_text;
if ( operationdetail .get_status() == STATUS_EXECUTE )
label_current_sub_text = operationdetail .get_description() ;
@ -182,7 +182,7 @@ void Dialog_Progress::update_gui_elements()
label_current_sub .set_markup( "<i>" + label_current_sub_text + "</i>\n" ) ;
//To ensure progress bar height remains the same, add a space in case message is empty
progressbar_current .set_text( operationdetail .progress_text + " " ) ;
progressbar_current.set_text( progress_text + " " );
}
bool Dialog_Progress::pulsebar_pulse()
@ -458,7 +458,7 @@ void Dialog_Progress::echo_operation_details( const OperationDetail & operationd
<< "<td>" << std::endl ;
for ( unsigned int t = 0 ; t < operationdetail .get_childs() .size() ; t++ )
echo_operation_details( operationdetail .get_childs()[ t ], out ) ;
echo_operation_details( *(operationdetail.get_childs()[ t ]), out );
out << "</td>" << std::endl << "</tr>" << std::endl ;
}

View File

@ -118,12 +118,12 @@ int FileSystem::execute_command( const Glib::ustring & command, OperationDetail
OperationDetail( output, STATUS_NONE, FONT_ITALIC ) );
operationdetail.get_last_child().add_child(
OperationDetail( error, STATUS_NONE, FONT_ITALIC ) );
std::vector<OperationDetail> &children = operationdetail.get_last_child().get_childs();
std::vector<OperationDetail*> &children = operationdetail.get_last_child().get_childs();
outputcapture.signal_update.connect( sigc::bind( sigc::ptr_fun( update_command_output ),
&(children[children.size() - 2]),
children[children.size() - 2],
&output ) );
errorcapture.signal_update.connect( sigc::bind( sigc::ptr_fun( update_command_output ),
&(children[children.size() - 1]),
children[children.size() - 1],
&error ) );
outputcapture.connect_signal();
errorcapture.connect_signal();

View File

@ -36,6 +36,11 @@ OperationDetail::OperationDetail( const Glib::ustring & description, OperationDe
OperationDetail::~OperationDetail()
{
cancelconnection.disconnect();
while (sub_details.size())
{
delete sub_details.back();
sub_details.pop_back();
}
}
void OperationDetail::set_description( const Glib::ustring & description, Font font )
@ -121,23 +126,23 @@ Glib::ustring OperationDetail::get_elapsed_time() const
void OperationDetail::add_child( const OperationDetail & operationdetail )
{
sub_details .push_back( operationdetail ) ;
sub_details .push_back( new OperationDetail(operationdetail) );
sub_details .back() .set_treepath( treepath + ":" + Utils::num_to_str( sub_details .size() -1 ) ) ;
sub_details .back() .signal_update .connect( sigc::mem_fun( this, &OperationDetail::on_update ) ) ;
sub_details.back().cancelconnection = signal_cancel.connect(
sigc::mem_fun( &sub_details.back(), &OperationDetail::cancel ) );
sub_details.back()->set_treepath( treepath + ":" + Utils::num_to_str( sub_details .size() -1 ) );
sub_details.back()->signal_update.connect( sigc::mem_fun( this, &OperationDetail::on_update ) );
sub_details.back()->cancelconnection = signal_cancel.connect(
sigc::mem_fun( sub_details.back(), &OperationDetail::cancel ) );
if ( cancelflag )
sub_details.back().cancel( cancelflag == 2 );
on_update( sub_details .back() ) ;
sub_details.back()->cancel( cancelflag == 2 );
on_update( *sub_details.back() );
}
std::vector<OperationDetail> & OperationDetail::get_childs()
std::vector<OperationDetail*> & OperationDetail::get_childs()
{
return sub_details ;
}
const std::vector<OperationDetail> & OperationDetail::get_childs() const
const std::vector<OperationDetail*> & OperationDetail::get_childs() const
{
return sub_details ;
}
@ -148,7 +153,7 @@ OperationDetail & OperationDetail::get_last_child()
if ( sub_details .size() == 0 )
add_child( OperationDetail( "---", STATUS_ERROR ) ) ;
return sub_details[sub_details.size() - 1];
return *sub_details[sub_details.size() - 1];
}
void OperationDetail::on_update( const OperationDetail & operationdetail )
@ -161,7 +166,8 @@ void OperationDetail::cancel( bool force )
{
if ( force )
cancelflag = 2;
else cancelflag = 1;
else
cancelflag = 1;
signal_cancel(force);
}