Add restart-and-run button. Fix update of server status line.

Author: Kasper Peeters

client_server/ComputeThread.cc

@@ -124,12 +124,17 @@ void ComputeThread::all_cells_nonrunning()
 
 void ComputeThread::on_fail(const boost::beast::error_code& ec)
 	{
-	std::cerr << "cadabra-client: connection to server on port " << port << " failed, " << ec.message() << std::endl;
+	if(!restarting_kernel) {
+		std::cerr << "cadabra-client: connection to server on port " << port << " failed, " << ec.message() << std::endl;
+		}
 	connection_is_open=false;
 	all_cells_nonrunning();
 	if(gui && server_pid!=0) {
+		// When a kernel restart is in progress, server_pid will be zero
+		// and this block never runs.
 		close(server_stdout);
 		// close(server_stderr);
+		// std::cerr << "closing connetion to terminated server" << std::endl;
 		Glib::spawn_close_pid(server_pid);
 		//		kill(server_pid, SIGKILL);
 		server_pid=0;
@@ -460,7 +465,8 @@ void ComputeThread::on_message(const std::string& msg)
 					   std::make_shared<ActionPositionCursor>(parent_id, ActionPositionCursor::Position::in);
 					docthread->queue_action(actionpos);
 
-					// FIXME: iterate over all cells and set the running flag to false.
+					// Action has stopped, so mark all cells as non-running.
+					all_cells_nonrunning();
 					}
 				else if (msg_type == "image_png") {
 					DataCell result(cell_id, DataCell::CellType::image_png, content["output"].get<std::string>());
@@ -525,7 +531,7 @@ void ComputeThread::execute_interactive(uint64_t id, const std::string& code)
 	std::ostringstream oss;
 	oss << req << std::endl;
 	if(getenv("CADABRA_SHOW_SENT")) {
-		std::cerr << "SEND: " << req.dump(3) << std::endl;
+		std::cerr << "SENT: " << req.dump(3) << std::endl;
 		}
 	wsclient.send(oss.str());
 	interactive_cells.insert(id);
@@ -593,7 +599,7 @@ void ComputeThread::execute_cell(DTree::iterator it, std::string no_assign, std:
 		std::ostringstream str;
 		str << req << std::endl;
 		if(getenv("CADABRA_SHOW_SENT")) {
-			std::cerr << "SEND: " << req.dump(3) << std::endl;
+			std::cerr << "SENT: " << req.dump(3) << std::endl;
 			}
 		wsclient.send(str.str());
 		// NOTE: we can get a return message in on_message at any point after this,
@@ -628,7 +634,7 @@ void ComputeThread::update_variable_on_server(std::string variable, double value
 	std::ostringstream str;
 	str << req << std::endl;
 	if(getenv("CADABRA_SHOW_SENT")) {
-		std::cerr << "SEND: " << req.dump(3) << std::endl;
+		std::cerr << "SENT: " << req.dump(3) << std::endl;
 		}
 	wsclient.send(str.str());
 	}
@@ -644,6 +650,8 @@ void ComputeThread::stop()
 	if(connection_is_open==false)
 		return;
 
+	// std::cerr << "stopping existing kernel" << std::endl;
+	
 	nlohmann::json req, header, content;
 	header["uuid"]="none";
 	header["msg_type"]="execute_interrupt";
@@ -657,10 +665,12 @@ void ComputeThread::stop()
 
 	server_pid=0;
 	if(getenv("CADABRA_SHOW_SENT")) {
-		std::cerr << "SEND: " << req.dump(3) << std::endl;
+		std::cerr << "SENT: " << req.dump(3) << std::endl;
 		}
 	wsclient.send(str.str());
-	all_cells_nonrunning();	
+	// Do not yet mark cells non-running, otherwise we are unable to
+	// process any error messages. Do this once the stop comes through.
+	// all_cells_nonrunning();	
 	}
 
 void ComputeThread::restart_kernel()
@@ -689,7 +699,7 @@ void ComputeThread::restart_kernel()
 	//	std::cerr << str.str() << std::endl;
 
 	if(getenv("CADABRA_SHOW_SENT")) {
-		std::cerr << "SEND: " << req.dump(3) << std::endl;
+		std::cerr << "SENT: " << req.dump(3) << std::endl;
 		}
 	wsclient.send(str.str());
 	docthread->on_interactive_output(req);

client_server/DocumentThread.cc

@@ -290,16 +290,16 @@ void DocumentThread::run_cells_referencing_variable(std::string variable, double
 
 void DocumentThread::process_action_queue()
 	{
-	// FIXME: we certainly do not want any two threads to run this at the same time,
-	// but that is not guaranteed. Actions should always be run on the GUI thread.
-	// This absolutely has to be run on the main GUI thread.
+	// This routine *absolutely* has to be run on the main GUI thread. Anything
+	// else is a bug.
 
 	if(main_thread_id != std::this_thread::get_id())
 		std::cerr << "INTERNAL ERROR: DocumentThread::process_action_queue not running on main thread."
 					 << std::endl;
 
 	stack_mutex.lock();
 	while(pending_actions.size()>0) {
+		// std::cerr << "pending_actions.size() == " << pending_actions.size() << std::endl;
 		std::shared_ptr<ActionBase> ab = pending_actions.front();
 		// Unlock the action queue while we are processing this particular action,
 		// so that other actions can be added which we run.

client_server/Server.cc

@@ -327,9 +327,9 @@ void Server::on_close(websocket_server::id_type ws_id)
 
 int quit(void *)
 	{
-	std::cerr << "Server: setting python interrupt." << std::endl;
+//	std::cerr << "Server: setting python interrupt." << std::endl;
 	PyErr_SetInterrupt();
-	std::cerr << "Server: python interrupt set." << std::endl;
+//	std::cerr << "Server: python interrupt set." << std::endl;
 	return -1;
 	}
 
@@ -441,7 +441,7 @@ void Server::wait_for_job()
 void Server::stop_block()
 	{
 //	interrupt_block=true;
-	std::cerr << "Server: sending SIGINT to python thread." << std::endl;
+//	std::cerr << "Server: sending SIGINT to python thread." << std::endl;
 	PyErr_SetInterrupt();
 
 	// PyGILState_STATE state = PyGILState_Ensure();
@@ -533,12 +533,12 @@ void Server::dispatch_message(websocket_server::id_type ws_id, const std::string
 		}
 	else if(msg_type=="execute_interrupt") {
 		std::unique_lock<std::mutex> lock(block_available_mutex);
-		std::cerr << "Server: requesting python thread stop." << std::endl;
+		// std::cerr << "Server: requesting python thread stop." << std::endl;
 		stop_block();
-		std::cerr << "Server: clearing block queue." << std::endl;
+		// std::cerr << "Server: clearing block queue." << std::endl;
 		std::queue<Block> empty;
 		std::swap(block_queue, empty);
-		std::cerr << "Server: block queue cleared." << std::endl;
+		// std::cerr << "Server: block queue cleared." << std::endl;
 		//snoop::log(snoop::warn) << "Job stop requested." << snoop::flush;
 		}
 	else if(msg_type=="init") {

frontend/gtkmm/CMakeLists.txt

@@ -250,6 +250,7 @@ set(CDBICONS
   cdb-copy-all
   cdb-open
   cdb-restart
+  cdb-restart-and-run-all  
   cdb-run
   cdb-save-as
   cdb-save

frontend/gtkmm/NotebookWindow.cc

@@ -73,7 +73,7 @@ NotebookWindow::NotebookWindow(Cadabra *c, bool ro, std::string geometry, std::s
 	, tex_running(false), tex_need_width(0)
 	, last_find_location(doc.end(), std::string::npos)
 	, is_configured(false)
-
+	, run_all_after_restart(false)
 	{
 	}
 
@@ -287,6 +287,7 @@ void NotebookWindow::on_realize()
 
 	// Kernel menu actions.
 	actiongroup->add_action( "KernelRestart",         sigc::mem_fun(*this, &NotebookWindow::on_kernel_restart) );			
+	actiongroup->add_action( "KernelRestartRun",      sigc::mem_fun(*this, &NotebookWindow::on_kernel_restart_and_run_all) );
 
 	// Tools menu actions.
 	actiongroup->add_action( "CompareFile",           sigc::mem_fun(*this, &NotebookWindow::compare_to_file) );
@@ -579,6 +580,10 @@ void NotebookWindow::on_realize()
 		"          <attribute name='label'>Restart kernel</attribute>"
 		"          <attribute name='action'>cdb.KernelRestart</attribute>"
 		"        </item>"
+	   "        <item>"
+		"          <attribute name='label'>Restart kernel and run notebook</attribute>"
+		"          <attribute name='action'>cdb.KernelRestartRun</attribute>"
+		"        </item>"
 		"      </section>"
 		"    </submenu>"
 		"    <submenu>"
@@ -708,6 +713,13 @@ void NotebookWindow::on_realize()
 								  false));
 		tool_restart.get_accessible()->set_name("Restart kernel");
 
+		// Restart-and-run-all button.
+		tool_restart_and_run_all.add(*Gtk::make_managed<ImageArea>(
+												  40/display_scale, display_scale,
+												  install_prefix()+"/share/cadabra2/cdb-icons/cdb-restart-and-run-all"+ICONEXT,
+												  false));
+		tool_restart_and_run_all.get_accessible()->set_name("Restart kernel and run notebook");
+
 		// Open button.
 		tool_open.add(*Gtk::make_managed<ImageArea>(
 								  40/display_scale, display_scale,
@@ -732,13 +744,15 @@ void NotebookWindow::on_realize()
 		tool_stop.set_size_request(70/display_scale, 70/display_scale);
 		tool_run.set_size_request(70/display_scale, 70/display_scale);
 		tool_restart.set_size_request(70/display_scale, 70/display_scale);
+		tool_restart_and_run_all.set_size_request(70/display_scale, 70/display_scale);
 		tool_open.set_size_request(70/display_scale, 70/display_scale);
 		tool_save.set_size_request(70/display_scale, 70/display_scale);
 		tool_save_as.set_size_request(70/display_scale, 70/display_scale);
 
 		tool_run.set_tooltip_text("Execute all cells");
 		tool_stop.set_tooltip_text("Stop execution");
 		tool_restart.set_tooltip_text("Restart kernel");
+		tool_restart_and_run_all.set_tooltip_text("Restart kernel and run entire notebook");
 		tool_open.set_tooltip_text("Open notebook...");
 		tool_save.set_tooltip_text("Save notebook");
 		tool_save_as.set_tooltip_text("Save notebook as...");
@@ -750,6 +764,7 @@ void NotebookWindow::on_realize()
 		toolbar.pack_start(tool_run, Gtk::PACK_SHRINK);
 		toolbar.pack_start(tool_stop, Gtk::PACK_SHRINK);
 		toolbar.pack_start(tool_restart, Gtk::PACK_SHRINK);
+		toolbar.pack_start(tool_restart_and_run_all, Gtk::PACK_SHRINK);
 		toolbar.pack_start(top_label);
 		toolbar.pack_end(kernel_spinner, Gtk::PACK_SHRINK);
 		kernel_spinner.set_size_request(50/display_scale, 50/display_scale);
@@ -766,6 +781,7 @@ void NotebookWindow::on_realize()
 	tool_run.signal_clicked().connect(sigc::mem_fun(*this, &NotebookWindow::run_all_cells));
 	tool_stop.signal_clicked().connect(sigc::mem_fun(*this, &NotebookWindow::on_run_stop));
 	tool_restart.signal_clicked().connect(sigc::mem_fun(*this, &NotebookWindow::on_kernel_restart));
+	tool_restart_and_run_all.signal_clicked().connect(sigc::mem_fun(*this, &NotebookWindow::on_kernel_restart_and_run_all));
 
 //	
 //	Gtk::Widget *toolbar=0;
@@ -1144,16 +1160,19 @@ void NotebookWindow::set_stop_sensitive(bool s)
 
 void NotebookWindow::process_data()
 	{
+	// std::cerr << "process_data() called" << std::endl;
 	dispatcher.emit();
 	}
 
 
 void NotebookWindow::on_connect()
 	{
+	// std::cerr << "kernel connected" << std::endl;
+	
 	std::lock_guard<std::mutex> guard(status_mutex);
 	kernel_string = "connected";
 	progress_string = "Idle";
-	dispatcher.emit();
+//	dispatcher.emit();
 	dispatch_update_status.emit();
 	console.initialize();
 	// prefs.python_path might end in a backslash which will raise an EOF syntax error, so we add a
@@ -1163,10 +1182,17 @@ void NotebookWindow::on_connect()
 	if (!name.empty()) {
 		console.send_input("sys.path.insert(0, '''" + escape_backslashes(name.substr(0, name.find_last_of("\\/"))) + "''')");
 		}
+
+	if(run_all_after_restart) {
+		run_all_after_restart=false;
+		std::shared_ptr<ActionBase> action = std::make_shared<ActionRunCell>();
+		queue_action(action);
+		}
 	}
 
 void NotebookWindow::on_disconnect(const std::string& reason)
 	{
+	// std::cerr << "***** kernel disconnected" << std::endl;
 	std::lock_guard<std::mutex> guard(status_mutex);
 	kernel_string = reason;
 	dispatcher.emit();
@@ -1191,6 +1217,7 @@ void NotebookWindow::on_kernel_runstatus(bool running)
 			follow_last_cell=doc.end();
 			}
 		}
+	// FIXME: WHY would we need to process the queue here?
 	dispatcher.emit();
 	}
 
@@ -1251,6 +1278,8 @@ void NotebookWindow::process_todo_queue()
 	{
 	static bool running=false;
 
+	// std::cerr << "process_todo_queue, running="  << running << std::endl;
+	
 	// Prevent from re-entering this from the process_action_queue entered below.
 	if(running) return;
 	running=true;
@@ -1631,6 +1660,7 @@ void NotebookWindow::set_progress(const std::string& msg, int cur_step, int tota
 void NotebookWindow::update_status()
 	{
 	// This should only be called from dispatch_update_status!
+	// This will then run on the GUI thread, as it should.
 
 	// Update status and progress, kernel status is taken card of in process_action_queue
 	std::lock_guard<std::mutex> guard(status_mutex);
@@ -2831,6 +2861,8 @@ void NotebookWindow::on_run_stop()
 void NotebookWindow::on_kernel_restart()
 	{
 	// FIXME: add warnings
+	kernel_string = "restarting";
+	update_status();
 	follow_last_cell=doc.end();
 
 	compute->restart_kernel();
@@ -2844,6 +2876,13 @@ void NotebookWindow::on_kernel_restart()
 	dispatch_update_status.emit();
 	}
 
+void NotebookWindow::on_kernel_restart_and_run_all()
+	{
+	// Run all cells after the restart has completed.
+	run_all_after_restart=true;
+	on_kernel_restart();
+	}
+
 void NotebookWindow::on_help() const
 	{
 	if(current_cell==doc.end()) return;

frontend/gtkmm/NotebookWindow.hh

@@ -56,11 +56,13 @@ namespace cadabra {
 
 			void           select_range(const DTree&, DTree::iterator, int start, int len);
 
+			// Implementations of the functions which the compute thread will
+			// call directly. If these need to modify the GUI, they need to do
+			// so by calling one of the dispatchers.
 			virtual void on_connect() override;
 			virtual void on_disconnect(const std::string&) override;
 			virtual void on_network_error() override;
 			virtual void on_kernel_runstatus(bool) override;
-
 			virtual void process_data() override;
 
 			// TeX stuff
@@ -146,7 +148,7 @@ namespace cadabra {
 			Gtk::Box                       topbox;
 			Gtk::Box                       toolbar;
 			Gtk::Button                    tool_open, tool_save, tool_save_as;
-			Gtk::Button                    tool_run, tool_run_to, tool_stop, tool_restart;		
+			Gtk::Button                    tool_run, tool_run_to, tool_stop, tool_restart, tool_restart_and_run_all;		
 			Gtk::Box                       supermainbox;
 			Gtk::Paned                     dragbox;
 			Gtk::Box                       mainbox;
@@ -184,7 +186,16 @@ namespace cadabra {
 			std::string                    status_string, kernel_string, progress_string;
 			double                         progress_frac;
 			int                            status_line, status_col;
+
+			// Functions which get called on the compute thread can signal to
+			// the GUI thread that elements need to be updated, by sending
+			// signals using the following dispatchers.
 			Glib::Dispatcher               dispatch_update_status, dispatch_refresh, dispatch_tex_error;
+
+			// Update the status line and progress bar. This should only
+			// be called on the GUI thread, so typically gets called
+			// indirectly by calling `dispatch_update_status.emmit()`
+			// from the compute thread, which calls this function.
 			void                           update_status();
 
 			// Run the TeX engine on a separate thread, then call
@@ -248,6 +259,7 @@ namespace cadabra {
 			void on_help() const;
 
 			void on_kernel_restart();
+			void on_kernel_restart_and_run_all();			
 
 			/// Search handling.
 			void on_search_text_changed();
@@ -263,6 +275,8 @@ namespace cadabra {
 			/// Todo deque processing logic. This gets called by the dispatcher, but it
 			/// is also allowed to call this from within NotebookWindow itself. The important
 			/// thing is that it is run on the GUI thread.
+			/// This is a wrapper around `Document::process_action_queue`, to set the
+			/// spinner status and handle crashes.
 			void process_todo_queue();
 
 			/// Refresh the display after a TeX engine run has completed. The TeX
@@ -321,7 +335,8 @@ namespace cadabra {
 			std::pair<DTree::iterator, size_t> last_find_location;
 			std::string                        last_find_string;
 
-			bool  is_configured;
+			bool  is_configured;         // have received and handled a configure event
+			bool  run_all_after_restart; // queue notebook running when kernel comes back
 
 			// We keep references to a few menu actions so we can
 			// enable/disable them at runtime.