2006-03-04 Dan Williams <dcbw@redhat.com>

Clean up activation cancellation.  Should be a lot faster now.  Observed
	an issue with wireless devices between stage 2 and 3 of activation, where
	activation would be cancelled, but the device thread wouldn't notice until
	the supplicant association timed out.  Reorganize activation such that
	a cancellation handler gets immediately scheduled in the device's thread,
	and devices have a chance to perform any custom cleanup too.

	* src/nm-device.[ch]
		- (activation_cancel_handler): new device-type-specific function
			for cleaning up device-type-specific stuff on cancellation
		- (cancel_activation): removed
		- (nm_device_activation_cancel): subsume functionality of
			real_cancel_activation, but instead of doing anything, punt
			operation to a handler that's run in device-thread context
		- (nm_device_schedule_activation_handle_cancel): fix spelling of
			a warning message
		- (activation_handle_cancel_helper): cancellation handler run in
			device-thread context, calls device-type-specific cancelation,
			then tears down the activation request
		- (real_activation_cancel_handler): generic cancellation handler,
			deals with cancelling any in-process DHCP request
		- (nm_device_activate_stage1_device_prepare,
		   nm_device_activate_stage2_device_config,
		   nm_device_activate_stage3_ip_config_start,
		   nm_device_activate_stage4_ip_config_get,
		   nm_device_activate_stage4_ip_config_timeout,
		   nm_device_activate_stage5_ip_commit): don't call
			nm_device_schedule_activation_handle_cancel() any more, since
			cancellation will have been already scheduled for us by
			nm_device_activation_cancel().  Just exit the function and
			assume that the cancel handler will be called next.

	* src/nm-device-802-3-ethernet.c
		- (real_act_stage2_config): remove; didn't do anything anyway

	* src/nm-device-802-11-wireless.c
		- (supplicant_status_cb): ensure we don't do anything if the activation
			got cancelled
		- (real_activation_cancel_handler): implement; cancel user key request
			on activation cancellation


git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@1549 4912f4e0-d625-0410-9fb7-b9a5a253dbdc

ChangeLog

@@ -1,3 +1,46 @@
+2006-03-04  Dan Williams  <dcbw@redhat.com>
+
+	Clean up activation cancellation.  Should be a lot faster now.  Observed
+	an issue with wireless devices between stage 2 and 3 of activation, where
+	activation would be cancelled, but the device thread wouldn't notice until
+	the supplicant association timed out.  Reorganize activation such that
+	a cancellation handler gets immediately scheduled in the device's thread,
+	and devices have a chance to perform any custom cleanup too.
+
+	* src/nm-device.[ch]
+		- (activation_cancel_handler): new device-type-specific function
+			for cleaning up device-type-specific stuff on cancellation
+		- (cancel_activation): removed
+		- (nm_device_activation_cancel): subsume functionality of
+			real_cancel_activation, but instead of doing anything, punt
+			operation to a handler that's run in device-thread context
+		- (nm_device_schedule_activation_handle_cancel): fix spelling of
+			a warning message
+		- (activation_handle_cancel_helper): cancellation handler run in
+			device-thread context, calls device-type-specific cancelation,
+			then tears down the activation request
+		- (real_activation_cancel_handler): generic cancellation handler,
+			deals with cancelling any in-process DHCP request
+		- (nm_device_activate_stage1_device_prepare,
+		   nm_device_activate_stage2_device_config,
+		   nm_device_activate_stage3_ip_config_start,
+		   nm_device_activate_stage4_ip_config_get,
+		   nm_device_activate_stage4_ip_config_timeout,
+		   nm_device_activate_stage5_ip_commit): don't call
+			nm_device_schedule_activation_handle_cancel() any more, since
+			cancellation will have been already scheduled for us by
+			nm_device_activation_cancel().  Just exit the function and
+			assume that the cancel handler will be called next.
+
+	* src/nm-device-802-3-ethernet.c
+		- (real_act_stage2_config): remove; didn't do anything anyway
+
+	* src/nm-device-802-11-wireless.c
+		- (supplicant_status_cb): ensure we don't do anything if the activation
+			got cancelled
+		- (real_activation_cancel_handler): implement; cancel user key request
+			on activation cancellation
+
 2006-03-04  Dan Williams  <dcbw@redhat.com>
 
 	* src/nm-device-802-11-wireless.c

src/nm-device-802-11-wireless.c

@@ -2192,6 +2192,12 @@ supplicant_status_cb (GIOChannel *source,
 
 	g_assert (self);
 
+	/* Do nothing if we're supposed to be canceling activation.
+	 * We'll get cleaned up by the cancellation handlers later.
+	 */
+	if (nm_device_activation_should_cancel (dev))
+		return TRUE;
+
 	ctrl = self->priv->supplicant.ctrl;
 	g_return_val_if_fail (ctrl != NULL, FALSE);
 
@@ -2830,6 +2836,27 @@ real_activation_failure_handler (NMDevice *dev,
 			ap ? nm_ap_get_essid (ap) : "(none)");
 }
 
+static void
+real_activation_cancel_handler (NMDevice *dev,
+                                NMActRequest *req)
+{
+	NMDevice80211Wireless *		self = NM_DEVICE_802_11_WIRELESS (dev);
+	NMDevice80211WirelessClass *	klass;
+	NMDeviceClass * 			parent_class;
+
+	/* Chain up to parent first */
+	klass = NM_DEVICE_802_11_WIRELESS_GET_CLASS (self);
+	parent_class = NM_DEVICE_CLASS (g_type_class_peek_parent (klass));
+	parent_class->activation_cancel_handler (dev, req);
+
+	if (nm_act_request_get_stage (req) == NM_ACT_STAGE_NEED_USER_KEY)
+	{
+		NMData *data = nm_device_get_app_data (dev);
+		nm_dbus_cancel_get_user_key_for_network (data->dbus_connection, req);
+	}
+}
+
+
 static gboolean
 real_can_interrupt_activation (NMDevice *dev)
 {
@@ -2922,6 +2949,7 @@ nm_device_802_11_wireless_class_init (NMDevice80211WirelessClass *klass)
 
 	parent_class->activation_failure_handler = real_activation_failure_handler;
 	parent_class->activation_success_handler = real_activation_success_handler;
+	parent_class->activation_cancel_handler = real_activation_cancel_handler;
 
 	g_type_class_add_private (object_class, sizeof (NMDevice80211WirelessPrivate));
 }

src/nm-device-802-3-ethernet.c

@@ -213,19 +213,6 @@ real_get_generic_capabilities (NMDevice *dev)
 	return caps;
 }
 
-static NMActStageReturn
-real_act_stage2_config (NMDevice *dev, NMActRequest *req)
-{
-	NMData *	data;
-
-	g_assert (req);
-	data = nm_act_request_get_data (req);
-	g_assert (data);
-
-	return TRUE;
-}
-
-
 static void
 nm_device_802_3_ethernet_dispose (GObject *object)
 {
@@ -280,8 +267,6 @@ nm_device_802_3_ethernet_class_init (NMDevice8023EthernetClass *klass)
 	parent_class->start = real_start;
 	parent_class->update_link = real_update_link;
 
-	parent_class->act_stage2_config = real_act_stage2_config;
-
 	g_type_class_add_private (object_class, sizeof (NMDevice8023EthernetPrivate));
 }
 

src/nm-device.c

@@ -75,9 +75,6 @@ static gpointer	nm_device_worker (gpointer user_data);
 
 static void		nm_device_activate_schedule_stage5_ip_config_commit (NMActRequest *req);
 
-static void		nm_device_schedule_activation_handle_cancel (NMActRequest *req);
-
-
 /*
  * nm_device_test_wireless_extensions
  *
@@ -678,10 +675,7 @@ nm_device_activate_stage1_device_prepare (NMActRequest *req)
 	g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS);
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	nm_device_activate_schedule_stage2_device_config (req);
 
@@ -762,10 +756,7 @@ nm_device_activate_stage2_device_config (NMActRequest *req)
 		nm_device_bring_up (self);
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	ret = NM_DEVICE_GET_CLASS (self)->act_stage2_config (self, req);
 	if (ret == NM_ACT_STAGE_RETURN_POSTPONE)
@@ -780,10 +771,7 @@ nm_device_activate_stage2_device_config (NMActRequest *req)
 	nm_info ("Activation (%s) Stage 2 of 5 (Device Configure) successful.", iface);
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	nm_device_activate_schedule_stage3_ip_config_start (req);
 
@@ -873,10 +861,7 @@ nm_device_activate_stage3_ip_config_start (NMActRequest *req)
 	nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) started...", iface);
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip_config_start (self, req);
 	if (ret == NM_ACT_STAGE_RETURN_POSTPONE)
@@ -889,10 +874,7 @@ nm_device_activate_stage3_ip_config_start (NMActRequest *req)
 	g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS);	
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	nm_device_activate_schedule_stage4_ip_config_get (req);
 
@@ -1019,18 +1001,12 @@ nm_device_activate_stage4_ip_config_get (NMActRequest *req)
 	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Get) started...", nm_device_get_iface (self));
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	ret = NM_DEVICE_GET_CLASS (self)->act_stage4_get_ip4_config (self, req, &ip4_config);
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	if (ret == NM_ACT_STAGE_RETURN_POSTPONE)
 		goto out;
@@ -1042,10 +1018,7 @@ nm_device_activate_stage4_ip_config_get (NMActRequest *req)
 	g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS);	
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	nm_act_request_set_ip4_config (req, ip4_config);
 	nm_device_activate_schedule_stage5_ip_config_commit (req);
@@ -1129,10 +1102,7 @@ nm_device_activate_stage4_ip_config_timeout (NMActRequest *req)
 	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Timeout) started...", iface);
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	ret = NM_DEVICE_GET_CLASS (self)->act_stage4_ip_config_timeout (self, req, &ip4_config);
 	if (ret == NM_ACT_STAGE_RETURN_POSTPONE)
@@ -1209,10 +1179,7 @@ nm_device_activate_stage5_ip_config_commit (NMActRequest *req)
 			nm_device_get_iface (self));
 
 	if (nm_device_activation_should_cancel (self))
-	{
-		nm_device_schedule_activation_handle_cancel (req);
 		goto out;
-	}
 
 	nm_device_set_ip4_config (self, ip4_config);
 	if (nm_system_device_set_from_ip4_config (self))
@@ -1262,27 +1229,39 @@ nm_device_activate_schedule_stage5_ip_config_commit (NMActRequest *req)
 }
 
 
+static void
+real_activation_cancel_handler (NMDevice *self,
+                                NMActRequest *req)
+{
+	g_return_if_fail (self != NULL);
+	g_return_if_fail (req != NULL);
+
+	if (nm_act_request_get_stage (req) == NM_ACT_STAGE_IP_CONFIG_START)
+		nm_dhcp_manager_cancel_transaction (self->priv->app_data->dhcp_manager, req);
+}
+
 /*
- * nm_device_activation_handle_cancel
+ * activation_handle_cancel_helper
  *
- * Cancel activation on a device and clean up.
+ * Allow specific device types to clean up their own cancellation
  *
  */
 static gboolean
-nm_device_activation_handle_cancel (NMActRequest *req)
+activation_handle_cancel_helper (NMActRequest *req)
 {
-	NMDevice *	self;
+	NMDevice * self;
-	NMData *		data;
+	NMDeviceClass *klass;
 
-	g_return_val_if_fail (req != NULL, FALSE);
+	g_assert (req);
-
-	data = nm_act_request_get_data (req);
-	g_assert (data);
 
 	self = nm_act_request_get_dev (req);
 	g_assert (self);
 
-	if ((req = nm_device_get_act_request (self)) && nm_device_is_activating (self))
+	klass = NM_DEVICE_CLASS (g_type_class_peek (NM_TYPE_DEVICE));
+	if (klass->activation_cancel_handler)
+		klass->activation_cancel_handler (self, req);
+
+	if ((req = nm_device_get_act_request (self)))
 	{
 		self->priv->act_request = NULL;
 		nm_act_request_unref (req);
@@ -1317,29 +1296,13 @@ nm_device_schedule_activation_handle_cancel (NMActRequest *req)
 
 	nm_info ("Activation (%s) cancellation handler scheduled...", nm_device_get_iface (self));
 	source = g_idle_source_new ();
-	g_source_set_callback (source, (GSourceFunc) nm_device_activation_handle_cancel, req, NULL);
+	g_source_set_callback (source, (GSourceFunc) activation_handle_cancel_helper, req, NULL);
+	g_source_set_priority (source, G_PRIORITY_HIGH_IDLE);
 	g_source_attach (source, self->priv->context);
 	g_source_unref (source);
 }
 
 
-/*
- * nm_device_activation_cancel
- *
- * Signal activation worker that it should stop and die.
- *
- */
-void
-nm_device_activation_cancel (NMDevice *self)
-{
-	NMDeviceClass *	klass;
-
-	g_return_if_fail (self != NULL);
-
-	klass = NM_DEVICE_CLASS (g_type_class_peek (NM_TYPE_DEVICE));
-	klass->cancel_activation (self);
-}
-
 static
 gboolean nm_ac_test (int tries,
                      nm_completion_args args)
@@ -1358,8 +1321,14 @@ gboolean nm_ac_test (int tries,
 	return TRUE;
 }
 
-static void
+/*
-real_cancel_activation (NMDevice *self)
+ * nm_device_activation_cancel
+ *
+ * Signal activation worker that it should stop and die.
+ *
+ */
+void
+nm_device_activation_cancel (NMDevice *self)
 {
 	nm_completion_args	args;
 	NMData *			app_data;
@@ -1372,28 +1341,11 @@ real_cancel_activation (NMDevice *self)
 	if (nm_device_is_activating (self))
 	{
 		NMActRequest *	req = nm_device_get_act_request (self);
-		gboolean		clear_act_request = FALSE;
 
 		nm_info ("Activation (%s): cancelling...", nm_device_get_iface (self));
 		self->priv->quit_activation = TRUE;
 
-		/* If the device is waiting for DHCP or a user key, force its current request to stop. */
+		nm_device_schedule_activation_handle_cancel (req);
-		if (nm_act_request_get_stage (req) == NM_ACT_STAGE_NEED_USER_KEY)
-		{
-			nm_dbus_cancel_get_user_key_for_network (app_data->dbus_connection, req);
-			clear_act_request = TRUE;
-		}
-		else if (nm_act_request_get_stage (req) == NM_ACT_STAGE_IP_CONFIG_START)
-		{
-			nm_dhcp_manager_cancel_transaction (app_data->dhcp_manager, req);
-			clear_act_request = TRUE;
-		}
-
-		if (clear_act_request)
-		{
-			self->priv->act_request = NULL;
-			nm_act_request_unref (req);
-		}
 
 		/* Spin until cancelled.  Possible race conditions or deadlocks here.
 		 * The other problem with waiting here is that we hold up dbus traffic
@@ -1972,7 +1924,7 @@ nm_device_class_init (NMDeviceClass *klass)
 	object_class->finalize = nm_device_finalize;
 
 	klass->is_test_device = real_is_test_device;
-	klass->cancel_activation = real_cancel_activation;
+	klass->activation_cancel_handler = real_activation_cancel_handler;
 	klass->get_type_capabilities = real_get_type_capabilities;
 	klass->get_generic_capabilities = real_get_generic_capabilities;
 	klass->start = real_start;

src/nm-device.h

@@ -102,12 +102,13 @@ struct _NMDeviceClass
 												 NMIP4Config **config);
 	void			(* deactivate)			(NMDevice *self);
 	void			(* deactivate_quickly)	(NMDevice *self);
-	void			(* cancel_activation)	(NMDevice *self);
 
 	void			(* activation_failure_handler)	(NMDevice *self,
 											 struct NMActRequest *req);
 	void			(* activation_success_handler)	(NMDevice *self,
 											 struct NMActRequest *req);
+	void			(* activation_cancel_handler)		(NMDevice *self,
+											 struct NMActRequest *req);
 
 	gboolean		(* can_interrupt_activation)		(NMDevice *self);
 };