Handle io watch error conditions gracefully

All calls to g_io_add_watch() request error condition reporting, and
all related callback functions at minimum will not create a virtual
busyloop by leaving the io watch active and ignoring the error state.

The mce io monitoring subsystem is modified so that instead of using
separate io watches for data and error handling + optional error
recovery callback it now has one io watch that first deals with
possible errors and then handles possible input via monitor type
specific logic and enforces delete notification callbacks for making
clean up at upper level logic. This should allow mce to gracefully
handle situations like adding/removing bluetooth keyboards.

[mce] Handle io watch error conditions gracefully

event-switches.c

@@ -424,6 +424,47 @@ static void submode_trigger(gconstpointer data)
 	old_submode = submode;
 }
 
+/** List of active io monitors for switches */
+static GSList *switch_iomon_list = NULL;
+
+/** I/O monitor delete callback
+ *
+ * @param iomon io monitor that is about to get deleted
+ */
+static void mce_switches_rem_iomon_cb(gconstpointer iomon)
+{
+	switch_iomon_list = g_slist_remove(switch_iomon_list, iomon);
+}
+
+/** Helper for adding io monitor for switch device
+ *
+ * @param path     device path
+ * @param input_cb input handler callback
+ *
+ * @return io monitor id, or NULL in case of errors
+ */
+static gconstpointer mce_switches_add_iomon(const char *path, iomon_cb input_cb)
+{
+	gconstpointer iomon =
+		mce_register_io_monitor_string(-1, path,
+					       MCE_IO_ERROR_POLICY_IGNORE,
+					       TRUE,
+					       input_cb,
+					       mce_switches_rem_iomon_cb);
+	if( iomon )
+		switch_iomon_list = g_slist_prepend(switch_iomon_list,
+						    (gpointer)iomon);
+
+	return iomon;
+}
+
+/** Unregister all active io monitors for switches
+ */
+static void mce_switches_rem_iomon_all(void)
+{
+	mce_unregister_io_monitor_list(switch_iomon_list);
+}
+
 /**
  * Init function for the switches component
  *
@@ -447,73 +488,49 @@ gboolean mce_switches_init(void)
 			       USE_INDATA, CACHE_INDATA);
 
 	/* Register I/O monitors */
-	// FIXME: error handling?
 	lockkey_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_FLICKER_KEY_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, lockkey_iomon_cb);
+		mce_switches_add_iomon(MCE_FLICKER_KEY_STATE_PATH,
+				       lockkey_iomon_cb);
+
 	kbd_slide_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_KBD_SLIDE_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, kbd_slide_iomon_cb);
+		mce_switches_add_iomon(MCE_KBD_SLIDE_STATE_PATH,
+				       kbd_slide_iomon_cb);
+
 	cam_focus_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_CAM_FOCUS_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, generic_activity_iomon_cb);
+		mce_switches_add_iomon(MCE_CAM_FOCUS_STATE_PATH,
+				       generic_activity_iomon_cb);
+
 	cam_launch_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_CAM_LAUNCH_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, camera_launch_button_iomon_cb);
+		mce_switches_add_iomon(MCE_CAM_LAUNCH_STATE_PATH,
+				       camera_launch_button_iomon_cb);
+
 	lid_cover_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_LID_COVER_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, lid_cover_iomon_cb);
+		mce_switches_add_iomon(MCE_LID_COVER_STATE_PATH,
+				       lid_cover_iomon_cb);
+
 	proximity_sensor_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_PROXIMITY_SENSOR_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, proximity_sensor_iomon_cb);
+		mce_switches_add_iomon(MCE_PROXIMITY_SENSOR_STATE_PATH,
+				       proximity_sensor_iomon_cb);
+
 	musb_omap3_usb_cable_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_MUSB_OMAP3_USB_CABLE_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, usb_cable_iomon_cb);
+		mce_switches_add_iomon(MCE_MUSB_OMAP3_USB_CABLE_STATE_PATH,
+				       usb_cable_iomon_cb);
+
 	lens_cover_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_LENS_COVER_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, lens_cover_iomon_cb);
+		mce_switches_add_iomon(MCE_LENS_COVER_STATE_PATH,
+				       lens_cover_iomon_cb);
+
 	mmc0_cover_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_MMC0_COVER_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, generic_activity_iomon_cb);
+		mce_switches_add_iomon(MCE_MMC0_COVER_STATE_PATH,
+				       generic_activity_iomon_cb);
+
 	mmc_cover_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_MMC_COVER_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, generic_activity_iomon_cb);
+		mce_switches_add_iomon(MCE_MMC_COVER_STATE_PATH,
+				       generic_activity_iomon_cb);
+
 	bat_cover_iomon_id =
-		mce_register_io_monitor_string(-1,
-					       MCE_BATTERY_COVER_STATE_PATH,
-					       MCE_IO_ERROR_POLICY_IGNORE,
-					       G_IO_PRI | G_IO_ERR,
-					       TRUE, generic_activity_iomon_cb);
+		mce_switches_add_iomon(MCE_BATTERY_COVER_STATE_PATH,
+				       generic_activity_iomon_cb);
 
 	update_proximity_monitor();
 
@@ -547,17 +564,7 @@ void mce_switches_exit(void)
 					   call_state_trigger);
 
 	/* Unregister I/O monitors */
-	mce_unregister_io_monitor(bat_cover_iomon_id);
-	mce_unregister_io_monitor(mmc_cover_iomon_id);
-	mce_unregister_io_monitor(mmc0_cover_iomon_id);
-	mce_unregister_io_monitor(lens_cover_iomon_id);
-	mce_unregister_io_monitor(musb_omap3_usb_cable_iomon_id);
-	mce_unregister_io_monitor(proximity_sensor_iomon_id);
-	mce_unregister_io_monitor(lid_cover_iomon_id);
-	mce_unregister_io_monitor(cam_launch_iomon_id);
-	mce_unregister_io_monitor(cam_focus_iomon_id);
-	mce_unregister_io_monitor(kbd_slide_iomon_id);
-	mce_unregister_io_monitor(lockkey_iomon_id);
+	mce_switches_rem_iomon_all();
 
 	return;
 }

filewatcher.c

@@ -341,14 +341,27 @@ filewatcher_input_cb(GIOChannel *source,
                      GIOCondition condition,
                      gpointer data)
 {
-  (void)source; (void)condition;
+  (void)source;
 
   filewatcher_t *self = data;
-  gboolean keep_going = filewatcher_process_events(self);
+  gboolean keep_going = TRUE;
+
+  if( condition & (G_IO_ERR | G_IO_HUP | G_IO_NVAL) )
+  {
+    keep_going = FALSE;
+  }
+
+  if( !filewatcher_process_events(self) )
+  {
+    keep_going = FALSE;
+  }
 
   if( !keep_going )
   {
-    mce_log(LL_WARN, "stopping inotify event io watch");
+    /* Note: This /should/ never happen, but if it does
+     *       we must not leave the io watch in a state
+     *       where it gets triggered forever. */
+    mce_log(LL_CRIT, "stopping inotify event io watch");
     self->watch_id = 0;
   }
 
@@ -435,7 +448,9 @@ filewatcher_setup_iowatch(filewatcher_t *self)
   }
   g_io_channel_set_buffered(chan, FALSE);
 
-  self->watch_id = g_io_add_watch(chan, G_IO_IN, filewatcher_input_cb, self);
+  self->watch_id = g_io_add_watch(chan,
+				  G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+				  filewatcher_input_cb, self);
 
   if( !self->watch_id )
   {

mce-dsme.c

@@ -106,10 +106,9 @@ static const mce_translation_t soft_poweroff_charger_connect_translation[] = {
 
 /** dsme I/O channel */
 static GIOChannel *dsme_iochan = NULL;
+
 /** dsme data channel GSource ID */
 static guint dsme_data_source_id;
-/** dsme error channel GSource ID */
-static guint dsme_error_source_id;
 
 static gboolean init_dsmesock(void);
 
@@ -121,7 +120,7 @@ static gboolean init_dsmesock(void);
  */
 static void mce_dsme_send(gpointer msg)
 {
-	if (dsme_disabled == TRUE)
+	if( dsme_disabled )
 		goto EXIT;
 
 	if (dsme_conn == NULL) {
@@ -423,8 +422,12 @@ static gboolean io_data_ready_cb(GIOChannel *source,
 	(void)condition;
 	(void)data;
 
-	if (dsme_disabled == TRUE)
-		goto EXIT;
+	if( condition & (G_IO_ERR | G_IO_HUP | G_IO_NVAL) ) {
+		mce_log(LL_CRIT, "DSME socket closed/error, exiting...");
+		// FIXME: this is not how one should exit from mainloop
+		mce_quit_mainloop();
+		exit(EXIT_FAILURE);
+	}
 
 	if ((msg = (dsmemsg_generic_t *)dsmesock_receive(dsme_conn)) == NULL)
 		goto EXIT;
@@ -490,35 +493,6 @@ static gboolean io_data_ready_cb(GIOChannel *source,
 	return TRUE;
 }
 
-/**
- * Callback for I/O errors from dsmesock
- *
- * @param source Unused
- * @param condition Unused
- * @param data Unused
- * @return Will never return; if there is an I/O-error we exit the mainloop
- */
-static gboolean io_error_cb(GIOChannel *source,
-			    GIOCondition condition,
-			    gpointer data) G_GNUC_NORETURN;
-
-static gboolean io_error_cb(GIOChannel *source,
-			    GIOCondition condition,
-			    gpointer data)
-{
-	/* Silence warnings */
-	(void)source;
-	(void)condition;
-	(void)data;
-
-	/* DSME socket closed/error */
-	mce_log(LL_CRIT,
-		"DSME socket closed/error, exiting...");
-	// FIXME: this is not how one should exit from mainloop
-	mce_quit_mainloop();
-	exit(EXIT_FAILURE);
-}
-
 /**
  * D-Bus callback for the init done notification signal
  *
@@ -587,11 +561,8 @@ static gboolean init_dsmesock(void)
 	}
 
 	dsme_data_source_id = g_io_add_watch(dsme_iochan,
-					     G_IO_IN | G_IO_PRI,
+					     G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
 					     io_data_ready_cb, NULL);
-	dsme_error_source_id = g_io_add_watch(dsme_iochan,
-					      G_IO_ERR | G_IO_HUP,
-					      io_error_cb, NULL);
 
 	/* Query the current system state; if the mainloop isn't running,
 	 * this will trigger an update when the mainloop starts
@@ -612,19 +583,26 @@ static void close_dsmesock(void)
 	mce_log(LL_DEBUG,
 		"Shutting down dsmesock I/O channel");
 
-	if (dsme_iochan != NULL) {
-		GError *error = NULL;
+	if( dsme_data_source_id ) {
 		g_source_remove(dsme_data_source_id);
-		g_source_remove(dsme_error_source_id);
+		dsme_data_source_id = 0;
+	}
+
+	if( dsme_iochan ) {
+		GError *error = NULL;
 		g_io_channel_shutdown(dsme_iochan, FALSE, &error);
 		g_io_channel_unref(dsme_iochan);
+		dsme_iochan = 0;
 		g_clear_error(&error);
 	}
 
 	mce_log(LL_DEBUG,
 		"Closing DSME sock");
 
-	dsmesock_close(dsme_conn);
+	if( dsme_conn ) {
+		dsmesock_close(dsme_conn);
+		dsme_conn = 0;
+	}
 }
 
 /**
@@ -645,12 +623,11 @@ gboolean mce_dsme_init(gboolean debug_mode)
 	mce_log(LL_DEBUG,
 		"Connecting to DSME sock");
 
-	if (init_dsmesock() == FALSE) {
-		if (debug_mode == TRUE) {
-			dsme_disabled = TRUE;
-		} else {
+	if( !init_dsmesock() ) {
+		if( !debug_mode )
 			goto EXIT;
-		}
+
+		dsme_disabled = TRUE;
 	}
 
 	/* Register with DSME's process watchdog */

mce-hybris.c

@@ -96,6 +96,11 @@ static gboolean evepipe_recv_cb(GIOChannel *channel,
 
   evepipe_t eve[64];
 
+  if( condition & (G_IO_ERR | G_IO_HUP | G_IO_NVAL) )
+  {
+    keep_going = FALSE;
+  }
+
   int rc = read(evepipe_fd[0], eve, sizeof eve);
 
   if( rc < 0 ) {
@@ -227,7 +232,9 @@ static bool evepipe_init(void)
     goto EXIT;
   }
 
-  evepipe_id = g_io_add_watch(chn, G_IO_IN, evepipe_recv_cb, 0);
+  evepipe_id = g_io_add_watch(chn,
+			      G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+			      evepipe_recv_cb, 0);
 
   if( !evepipe_id ) {
     goto EXIT;