[PATCH 0/5] Clean up atoms on modem_unregister

[PATCH 0/5] Clean up atoms on modem_unregister

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

From: John Ernberg <john.ernberg@actia.se>

When using e.g. a USB modem and it's unplugged oFono may crash with a
SIGSEGV or SIGFPE due to atoms not getting cleaned up properly.

John Ernberg (5):
  modem: clean up atoms on modem_unregister
  bluez4: track and clean up atom watches
  bluez5: track and clean up atom watches
  push notification: track and clean up atom watches
  smart messaging: track and clean up atom watches

 plugins/dun_gw_bluez4.c     | 24 ++++++++++++++++++++++--
 plugins/dun_gw_bluez5.c     | 24 ++++++++++++++++++++++--
 plugins/hfp_ag_bluez4.c     | 25 ++++++++++++++++++++++---
 plugins/hfp_ag_bluez5.c     | 36 +++++++++++++++++++++++++++++++-----
 plugins/push-notification.c | 26 +++++++++++++++++++++++---
 plugins/smart-messaging.c   | 26 +++++++++++++++++++++++---
 src/modem.c                 |  3 +++
 7 files changed, 146 insertions(+), 18 deletions(-)

-- 
1.9.1

[PATCH 1/5] modem: clean up atoms on modem_unregister

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

From: John Ernberg <john.ernberg@actia.se>

This resolves a crash that can happen when a e.g. usb modem is removed.
---
 src/modem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/modem.c b/src/modem.c
index a89fa48..4968839 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -2071,6 +2071,9 @@ static void modem_unregister(struct ofono_modem *modem)
 	if (modem->powered == TRUE)
 		set_powered(modem, FALSE);
 
+	if (modem->atoms)
+		flush_atoms(modem, MODEM_STATE_POWER_OFF);
+
 	__ofono_watchlist_free(modem->atom_watches);
 	modem->atom_watches = NULL;
 
-- 
1.9.1

[PATCH 2/5] bluez4: track and clean up atom watches

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 4430 bytes --]

From: John Ernberg <john.ernberg@actia.se>

Prevents glib from causing SIGFPE during certain circumstances of modem
removal.
---
 plugins/dun_gw_bluez4.c | 24 ++++++++++++++++++++++--
 plugins/hfp_ag_bluez4.c | 25 ++++++++++++++++++++++---
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/plugins/dun_gw_bluez4.c b/plugins/dun_gw_bluez4.c
index a1de7a4..570da7f 100644
--- a/plugins/dun_gw_bluez4.c
+++ b/plugins/dun_gw_bluez4.c
@@ -40,6 +40,7 @@
 static struct server *server;
 static guint modemwatch_id;
 static GList *modems;
+static GHashTable *gprs_watches = NULL;
 
 static const gchar *dun_record =
 "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
@@ -129,15 +130,29 @@ static void gprs_watch(struct ofono_atom *atom,
 	}
 }
 
+static gboolean atom_watch_remove(gpointer key, gpointer value,
+					gpointer user_data)
+{
+	struct ofono_modem *modem = key;
+
+	__ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
+
+	return TRUE;
+}
+
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
 {
+	int gprs;
 	DBG("modem: %p, added: %d", modem, added);
 
-	if (added == FALSE)
+	if (added == FALSE) {
+		g_hash_table_remove(gprs_watches, modem);
 		return;
+	}
 
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_GPRS,
+	gprs = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_GPRS,
 						gprs_watch, modem, NULL);
+	g_hash_table_insert(gprs_watches, modem, GUINT_TO_POINTER(gprs));
 }
 
 static void call_modemwatch(struct ofono_modem *modem, void *user)
@@ -149,6 +164,8 @@ static int dun_gw_init(void)
 {
 	DBG("");
 
+	gprs_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
+
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 
 	__ofono_modem_foreach(call_modemwatch, NULL);
@@ -161,6 +178,9 @@ static void dun_gw_exit(void)
 	__ofono_modemwatch_remove(modemwatch_id);
 	g_list_free(modems);
 
+	g_hash_table_foreach_remove(gprs_watches, atom_watch_remove, NULL);
+	g_hash_table_destroy(gprs_watches);
+
 	if (server) {
 		bluetooth_unregister_server(server);
 		server = NULL;
diff --git a/plugins/hfp_ag_bluez4.c b/plugins/hfp_ag_bluez4.c
index 039b665..b931ae1 100644
--- a/plugins/hfp_ag_bluez4.c
+++ b/plugins/hfp_ag_bluez4.c
@@ -41,6 +41,7 @@ static struct server *server;
 static guint modemwatch_id;
 static GList *modems;
 static GHashTable *sim_hash = NULL;
+static GHashTable *sim_watches = NULL;
 
 static const gchar *hfp_ag_record =
 "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
@@ -142,6 +143,16 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *data)
 						hfp_ag_connect_cb, NULL);
 }
 
+static gboolean atom_watch_remove(gpointer key, gpointer value,
+					gpointer user_data)
+{
+	struct ofono_modem *modem = key;
+
+	__ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
+
+	return TRUE;
+}
+
 static gboolean sim_watch_remove(gpointer key, gpointer value,
 				gpointer user_data)
 {
@@ -176,13 +187,17 @@ static void sim_watch(struct ofono_atom *atom,
 
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
 {
+	int sim;
 	DBG("modem: %p, added: %d", modem, added);
 
-	if (added == FALSE)
+	if (added == FALSE) {
+		g_hash_table_remove(sim_watches, modem);
 		return;
+	}
 
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SIM,
-					sim_watch, modem, NULL);
+	sim = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SIM,
+						sim_watch, modem, NULL);
+	g_hash_table_insert(sim_watches, modem, GUINT_TO_POINTER(sim));
 }
 
 static void call_modemwatch(struct ofono_modem *modem, void *user)
@@ -193,6 +208,7 @@ static void call_modemwatch(struct ofono_modem *modem, void *user)
 static int hfp_ag_init(void)
 {
 	sim_hash = g_hash_table_new(g_direct_hash, g_direct_equal);
+	sim_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
 
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 	__ofono_modem_foreach(call_modemwatch, NULL);
@@ -207,6 +223,9 @@ static void hfp_ag_exit(void)
 	g_hash_table_foreach_remove(sim_hash, sim_watch_remove, NULL);
 	g_hash_table_destroy(sim_hash);
 
+	g_hash_table_foreach_remove(sim_watches, atom_watch_remove, NULL);
+	g_hash_table_destroy(sim_watches);
+
 	if (server) {
 		bluetooth_unregister_server(server);
 		server = NULL;
-- 
1.9.1

[PATCH 3/5] bluez5: track and clean up atom watches

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 5252 bytes --]

From: John Ernberg <john.ernberg@actia.se>

Prevents glib from causing SIGFPE during certain circumstances of modem
removal.
---
 plugins/dun_gw_bluez5.c | 24 ++++++++++++++++++++++--
 plugins/hfp_ag_bluez5.c | 36 +++++++++++++++++++++++++++++++-----
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/plugins/dun_gw_bluez5.c b/plugins/dun_gw_bluez5.c
index faea12b..af78795 100644
--- a/plugins/dun_gw_bluez5.c
+++ b/plugins/dun_gw_bluez5.c
@@ -48,6 +48,7 @@
 
 static guint modemwatch_id;
 static GList *modems;
+static GHashTable *gprs_watches = NULL;
 
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 						DBusMessage *msg, void *data)
@@ -151,6 +152,16 @@ static const GDBusMethodTable profile_methods[] = {
 	{ }
 };
 
+static gboolean atom_watch_remove(gpointer key, gpointer value,
+					gpointer user_data)
+{
+	struct ofono_modem *modem = key;
+
+	__ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
+
+	return TRUE;
+}
+
 static void gprs_watch(struct ofono_atom *atom,
 				enum ofono_atom_watch_condition cond,
 				void *data)
@@ -177,13 +188,17 @@ static void gprs_watch(struct ofono_atom *atom,
 
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
 {
+	int gprs;
 	DBG("modem: %p, added: %d", modem, added);
 
-	if (added == FALSE)
+	if (added == FALSE) {
+		g_hash_table_remove(gprs_watches, modem);
 		return;
+	}
 
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_GPRS,
+	gprs = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_GPRS,
 						gprs_watch, modem, NULL);
+	g_hash_table_insert(gprs_watches, modem, GUINT_TO_POINTER(gprs));
 }
 
 static void call_modemwatch(struct ofono_modem *modem, void *user)
@@ -210,6 +225,8 @@ static int dun_gw_init(void)
 		return -EIO;
 	}
 
+	gprs_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
+
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 
 	__ofono_modem_foreach(call_modemwatch, NULL);
@@ -228,6 +245,9 @@ static void dun_gw_exit(void)
 	bt_unregister_profile(conn, DUN_GW_EXT_PROFILE_PATH);
 	g_dbus_unregister_interface(conn, DUN_GW_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
+
+	g_hash_table_foreach_remove(gprs_watches, atom_watch_remove, NULL);
+	g_hash_table_destroy(gprs_watches);
 }
 
 OFONO_PLUGIN_DEFINE(dun_gw_bluez5, "Dial-up Networking Profile Plugins",
diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index 22faeb7..ba0f802 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -60,6 +60,8 @@ static guint modemwatch_id;
 static GList *modems;
 static GHashTable *sim_hash = NULL;
 static GHashTable *connection_hash;
+static GHashTable *sim_atom_watches = NULL;
+static GHashTable *vc_atom_watches = NULL;
 
 static int hfp_card_probe(struct ofono_handsfree_card *card,
 					unsigned int vendor, void *data)
@@ -374,6 +376,16 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *data)
 					HFP_AG_EXT_PROFILE_PATH, NULL, 0);
 }
 
+static gboolean atom_watch_remove(gpointer key, gpointer value,
+					gpointer user_data)
+{
+	struct ofono_modem *modem = key;
+
+	__ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
+
+	return TRUE;
+}
+
 static gboolean sim_watch_remove(gpointer key, gpointer value,
 				gpointer user_data)
 {
@@ -443,15 +455,21 @@ static void voicecall_watch(struct ofono_atom *atom,
 
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
 {
+	int sim, vc;
 	DBG("modem: %p, added: %d", modem, added);
 
-	if (added == FALSE)
+	if (added == FALSE) {
+		g_hash_table_remove(sim_atom_watches, modem);
+		g_hash_table_remove(vc_atom_watches, modem);
 		return;
+	}
 
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SIM,
-					sim_watch, modem, NULL);
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_VOICECALL,
-					voicecall_watch, modem, NULL);
+	sim = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SIM,
+						sim_watch, modem, NULL);
+	vc = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_VOICECALL,
+						voicecall_watch, modem, NULL);
+	g_hash_table_insert(sim_atom_watches, modem, GUINT_TO_POINTER(sim));
+	g_hash_table_insert(vc_atom_watches, modem, GUINT_TO_POINTER(vc));
 }
 
 static void call_modemwatch(struct ofono_modem *modem, void *user)
@@ -485,6 +503,8 @@ static int hfp_ag_init(void)
 	}
 
 	sim_hash = g_hash_table_new(g_direct_hash, g_direct_equal);
+	sim_atom_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
+	vc_atom_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
 
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 	__ofono_modem_foreach(call_modemwatch, NULL);
@@ -513,6 +533,12 @@ static void hfp_ag_exit(void)
 	g_hash_table_foreach_remove(sim_hash, sim_watch_remove, NULL);
 	g_hash_table_destroy(sim_hash);
 
+	g_hash_table_foreach_remove(sim_atom_watches, atom_watch_remove, NULL);
+	g_hash_table_destroy(sim_atom_watches);
+
+	g_hash_table_foreach_remove(vc_atom_watches, atom_watch_remove, NULL);
+	g_hash_table_destroy(vc_atom_watches);
+
 	ofono_handsfree_audio_unref();
 }
 
-- 
1.9.1

[PATCH 4/5] push notification: track and clean up atom watches

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]

From: John Ernberg <john.ernberg@actia.se>

Prevents glib from causing SIGFPE during certain circumstances of modem
removal.
---
 plugins/push-notification.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/plugins/push-notification.c b/plugins/push-notification.c
index ff388d9..ecf100f 100644
--- a/plugins/push-notification.c
+++ b/plugins/push-notification.c
@@ -45,6 +45,7 @@
 #define WAP_PUSH_DST_PORT 2948
 
 static unsigned int modemwatch_id;
+static GHashTable *sms_watches = NULL;
 
 struct push_notification {
 	struct ofono_modem *modem;
@@ -164,6 +165,16 @@ static void push_notification_cleanup(gpointer user)
 	ofono_modem_remove_interface(pn->modem, PUSH_NOTIFICATION_INTERFACE);
 }
 
+static gboolean atom_watch_remove(gpointer key, gpointer value,
+					gpointer user_data)
+{
+	struct ofono_modem *modem = key;
+
+	__ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
+
+	return TRUE;
+}
+
 static void sms_watch(struct ofono_atom *atom,
 				enum ofono_atom_watch_condition cond,
 				void *data)
@@ -197,18 +208,22 @@ static void sms_watch(struct ofono_atom *atom,
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
 {
 	struct push_notification *pn;
+	int sms;
 	DBG("modem: %p, added: %d", modem, added);
 
-	if (added == FALSE)
+	if (added == FALSE) {
+		g_hash_table_remove(sms_watches, modem);
 		return;
+	}
 
 	pn = g_try_new0(struct push_notification, 1);
 	if (pn == NULL)
 		return;
 
 	pn->modem = modem;
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
-					sms_watch, pn, g_free);
+	sms = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
+						sms_watch, pn, g_free);
+	g_hash_table_insert(sms_watches, modem, GUINT_TO_POINTER(sms));
 }
 
 static void call_modemwatch(struct ofono_modem *modem, void *user)
@@ -220,6 +235,8 @@ static int push_notification_init(void)
 {
 	DBG("");
 
+	sms_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
+
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 
 	__ofono_modem_foreach(call_modemwatch, NULL);
@@ -232,6 +249,9 @@ static void push_notification_exit(void)
 	DBG("");
 
 	__ofono_modemwatch_remove(modemwatch_id);
+
+	g_hash_table_foreach_remove(sms_watches, atom_watch_remove, NULL);
+	g_hash_table_destroy(sms_watches);
 }
 
 OFONO_PLUGIN_DEFINE(push_notification, "Push Notification Plugin", VERSION,
-- 
1.9.1

[PATCH 5/5] smart messaging: track and clean up atom watches

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 2406 bytes --]

From: John Ernberg <john.ernberg@actia.se>

Prevents glib from causing SIGFPE during certain circumstances of modem
removal.
---
 plugins/smart-messaging.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/plugins/smart-messaging.c b/plugins/smart-messaging.c
index b368917..38a03f3 100644
--- a/plugins/smart-messaging.c
+++ b/plugins/smart-messaging.c
@@ -48,6 +48,7 @@
 #define VCAL_SRC_PORT -1
 #define VCAL_DST_PORT 9205
 
+static GHashTable *sms_watches = NULL;
 static unsigned int modemwatch_id;
 
 struct smart_messaging {
@@ -284,6 +285,16 @@ static const GDBusMethodTable smart_messaging_methods[] = {
 	{ }
 };
 
+static gboolean atom_watch_remove(gpointer key, gpointer value,
+					gpointer user_data)
+{
+	struct ofono_modem *modem = key;
+
+	__ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
+
+	return TRUE;
+}
+
 static void smart_messaging_cleanup(gpointer user)
 {
 	struct smart_messaging *sm = user;
@@ -333,18 +344,22 @@ static void sms_watch(struct ofono_atom *atom,
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
 {
 	struct smart_messaging *sm;
+	int sms;
 	DBG("modem: %p, added: %d", modem, added);
 
-	if (added == FALSE)
+	if (added == FALSE) {
+		g_hash_table_remove(sms_watches, modem);
 		return;
+	}
 
 	sm = g_try_new0(struct smart_messaging, 1);
 	if (sm == NULL)
 		return;
 
 	sm->modem = modem;
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
-					sms_watch, sm, g_free);
+	sms = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
+						sms_watch, sm, g_free);
+	g_hash_table_insert(sms_watches, modem, GUINT_TO_POINTER(sms));
 }
 
 static void call_modemwatch(struct ofono_modem *modem, void *user)
@@ -356,6 +371,8 @@ static int smart_messaging_init(void)
 {
 	DBG("");
 
+	sms_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
+
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 
 	__ofono_modem_foreach(call_modemwatch, NULL);
@@ -368,6 +385,9 @@ static void smart_messaging_exit(void)
 	DBG("");
 
 	__ofono_modemwatch_remove(modemwatch_id);
+
+	g_hash_table_foreach_remove(sms_watches, atom_watch_remove, NULL);
+	g_hash_table_destroy(sms_watches);
 }
 
 OFONO_PLUGIN_DEFINE(smart_messaging, "Smart Messaging Plugin", VERSION,
-- 
1.9.1

Re: [PATCH 4/5] push notification: track and clean up atom watches

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 3077 bytes --]

Hi John,

On 04/22/2016 08:07 AM, John Ernberg wrote:
> From: John Ernberg <john.ernberg@actia.se>
>
> Prevents glib from causing SIGFPE during certain circumstances of modem
> removal.

Do you have a stack trace handy?

> ---
>   plugins/push-notification.c | 26 +++++++++++++++++++++++---
>   1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/plugins/push-notification.c b/plugins/push-notification.c
> index ff388d9..ecf100f 100644
> --- a/plugins/push-notification.c
> +++ b/plugins/push-notification.c
> @@ -45,6 +45,7 @@
>   #define WAP_PUSH_DST_PORT 2948
>
>   static unsigned int modemwatch_id;
> +static GHashTable *sms_watches = NULL;
>
>   struct push_notification {
>   	struct ofono_modem *modem;
> @@ -164,6 +165,16 @@ static void push_notification_cleanup(gpointer user)
>   	ofono_modem_remove_interface(pn->modem, PUSH_NOTIFICATION_INTERFACE);
>   }
>
> +static gboolean atom_watch_remove(gpointer key, gpointer value,
> +					gpointer user_data)
> +{
> +	struct ofono_modem *modem = key;
> +
> +	__ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
> +
> +	return TRUE;
> +}
> +
>   static void sms_watch(struct ofono_atom *atom,
>   				enum ofono_atom_watch_condition cond,
>   				void *data)
> @@ -197,18 +208,22 @@ static void sms_watch(struct ofono_atom *atom,
>   static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
>   {
>   	struct push_notification *pn;
> +	int sms;
>   	DBG("modem: %p, added: %d", modem, added);
>
> -	if (added == FALSE)
> +	if (added == FALSE) {
> +		g_hash_table_remove(sms_watches, modem);
>   		return;
> +	}

This has no effect.  The atom_watches for that particular modem have 
already been freed by the time the modem watch has been called in 
call_modemwatches().  See modem_unregister for details.

>
>   	pn = g_try_new0(struct push_notification, 1);
>   	if (pn == NULL)
>   		return;
>
>   	pn->modem = modem;
> -	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
> -					sms_watch, pn, g_free);
> +	sms = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
> +						sms_watch, pn, g_free);
> +	g_hash_table_insert(sms_watches, modem, GUINT_TO_POINTER(sms));
>   }
>
>   static void call_modemwatch(struct ofono_modem *modem, void *user)
> @@ -220,6 +235,8 @@ static int push_notification_init(void)
>   {
>   	DBG("");
>
> +	sms_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
> +
>   	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
>
>   	__ofono_modem_foreach(call_modemwatch, NULL);
> @@ -232,6 +249,9 @@ static void push_notification_exit(void)
>   	DBG("");
>
>   	__ofono_modemwatch_remove(modemwatch_id);
> +
> +	g_hash_table_foreach_remove(sms_watches, atom_watch_remove, NULL);
> +	g_hash_table_destroy(sms_watches);

atom watches are already being removed by the virtue of modems being 
unregistered.

>   }
>
>   OFONO_PLUGIN_DEFINE(push_notification, "Push Notification Plugin", VERSION,
>

Regards,
-Denis

Re: [PATCH 4/5] push notification: track and clean up atom watches

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 15910 bytes --]

Hi Denis,

On 04/22/2016 09:43 PM, Denis Kenzior wrote:
> Hi John,
>
> On 04/22/2016 08:07 AM, John Ernberg wrote:
>> From: John Ernberg <john.ernberg@actia.se>
>>
>> Prevents glib from causing SIGFPE during certain circumstances of modem
>> removal.
>
> Do you have a stack trace handy?
I have one for the bluez5 hfp plugin which was recreated by stopping 
oFono, the rest of the changes were added as a sort of safeguard.

Stack trace:

(gdb)
(gdb) bt
#0  0x76d88748 in raise (sig=8)
     at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
#1  0x76e8bcac in __aeabi_ldiv0 ()
     at 
/opt/yocto/build/tmp/work-shared/gcc-4.8.2-r0/gcc-4.8.2/libgcc/config/arm/lib1funcs.S:1331
#2  0x76e0e1b4 in g_hash_table_lookup_node (hash_return=<synthetic 
pointer>,
     key=0xe17418, hash_table=0xe10e38)
     at /usr/src/debug/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/ghash.c:371
#3  g_hash_table_lookup (hash_table=0xe10e38, key=key(a)entry=0xe17418)
     at /usr/src/debug/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/ghash.c:1076
#4  0x0008ccb8 in sim_watch (atom=<optimized out>, cond=<optimized out>,
     data=0xe18f88) at plugins/hfp_ag_bluez5.c:260
#5  0x000910d0 in call_watches (atom=atom(a)entry=0xe17540,
     cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at 
src/modem.c:261
#6  0x00091de0 in __ofono_atom_unregister (atom=atom(a)entry=0xe17540)
     at src/modem.c:281
#7  0x00091e54 in flush_atoms (
     new_state=new_state(a)entry=MODEM_STATE_POWER_OFF, modem=0xe18f88)
     at src/modem.c:430
#8  0x00091f60 in modem_change_state (modem=0xe18f88,
     new_state=MODEM_STATE_POWER_OFF) at src/modem.c:511
#9  0x000921c8 in set_powered (modem=modem(a)entry=0xe18f88,
     powered=powered(a)entry=0) at src/modem.c:882
#10 0x00092990 in modem_unregister (modem=0xe18f88) at src/modem.c:2040
#11 0x00093cec in ofono_modem_driver_unregister (d=0x112168)
     at src/modem.c:2176
#12 0x00091040 in __ofono_plugin_cleanup () at src/plugin.c:197
#13 0x00014e24 in main (argc=85540, argv=0x7ee60d44) at src/main.c:255

oFono debug log:

ofonod[5126]: src/plugin.c:__ofono_plugin_cleanup()
ofonod[5126]: plugins/push-notification.c:push_notification_exit()
ofonod[5126]: plugins/smart-messaging.c:smart_messaging_exit()
ofonod[5126]: plugins/bluez5.c:bt_unregister_profile() Bluetooth: 
Unregistering profile /bluetooth/profile/dun_gw
ofonod[5126]: plugins/bluez5.c:bt_unregister_profile() Bluetooth: 
Unregistering profile /bluetooth/profile/hfp_hf
ofonod[5126]: 
src/handsfree-audio.c:ofono_handsfree_card_driver_unregister() driver: 
0x113994
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1139a4, name: hfp
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x11391c, name: ublox
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1138b4, name: quectel
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113854, name: he910
ofonod[5126]: 
src/private-network.c:ofono_private_network_driver_unregister() driver: 
0x113828, name: ConnMan Private Network
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1137d0, name: sim900
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113778, name: samsung
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113720, name: speedupcdma
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1136c0, name: speedup
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113668, name: alcatel
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113600, name: icera
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1135a8, name: linktop
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113550, name: nokiacdma
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1134f8, name: nokia
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1134a0, name: tc65
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113408, name: ste
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1133a0, name: ifx
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113348, name: palmpre
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1132e8, name: novatel
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113290, name: sierra
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113208, name: huawei
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1131b0, name: zte
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113140, name: hso
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x1130e0, name: mbm
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113080, name: calypso
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x113028, name: wavecom
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x112fd0, name: g1
ofonod[5126]: 
src/cdma-voicecall.c:ofono_cdma_voicecall_driver_unregister() driver: 
0x112f78, name: cdmamodem
ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver: 
0x112fa0, name: cdmamodem
ofonod[5126]: src/cdma-connman.c:ofono_cdma_connman_driver_unregister() 
driver: 0x112fbc, name: cdmamodem
ofonod[5126]: src/ctm.c:ofono_ctm_driver_unregister() driver: 0x112e88, 
name: phonesim
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x112e9c, name: phonesim
ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver: 
0x112eb8, name: phonesim
ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver: 
0x112e74, name: speedupmodem
ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister() 
driver: 0x112d38, name: hfpmodem
ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver: 
0x112ddc, name: hfpmodem
ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver: 
0x112d90, name: hfpmodem
ofonod[5126]: src/call-volume.c:ofono_call_volume_driver_unregister() 
driver: 0x112dc4, name: hfpmodem
ofonod[5126]: src/handsfree.c:ofono_handsfree_driver_unregister() 
driver: 0x112e08, name: hfpmodem
ofonod[5126]: src/siri.c:ofono_siri_driver_unregister() driver: 
0x112e3c, name: hfpmodem
ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver: 
0x112cb8, name: dunmodem
ofonod[5126]: src/gprs.c:ofono_gprs_driver_unregister() driver: 
0x112cdc, name: dunmodem
ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister() 
driver: 0x112bec, name: stemodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x112c74, name: stemodem
ofonod[5126]: 
src/radio-settings.c:ofono_radio_settings_driver_unregister() driver: 
0x112c3c, name: stemodem
ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112b98, 
name: ifxmodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x112b6c, name: ifxmodem
ofonod[5126]: 
src/radio-settings.c:ofono_radio_settings_driver_unregister() driver: 
0x112b38, name: ifxmodem
ofonod[5126]: 
src/audio-settings.c:ofono_audio_settings_driver_unregister() driver: 
0x112b24, name: ifxmodem
ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister() 
driver: 0x112acc, name: ifxmodem
ofonod[5126]: src/ctm.c:ofono_ctm_driver_unregister() driver: 0x112bb8, 
name: ifxmodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x112a5c, name: hsomodem
ofonod[5126]: 
src/radio-settings.c:ofono_radio_settings_driver_unregister() driver: 
0x112a80, name: hsomodem
ofonod[5126]: 
src/location-reporting.c:ofono_location_reporting_driver_unregister() 
driver: 0x112a1c, name: mbmmodem
ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x1129fc, 
name: mbmmodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x1129d8, name: mbmmodem
ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112990, 
name: calypsomodem
ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister() 
driver: 0x112940, name: calypsomodem
ofonod[5126]: src/cdma-netreg.c:ofono_cdma_netreg_driver_unregister() 
driver: 0x112910, name: huaweimodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x1128c0, name: huaweimodem
ofonod[5126]: 
src/radio-settings.c:ofono_radio_settings_driver_unregister() driver: 
0x1128e4, name: huaweimodem
ofonod[5126]: 
src/audio-settings.c:ofono_audio_settings_driver_unregister() driver: 
0x1128ac, name: huaweimodem
ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister() 
driver: 0x11285c, name: huaweimodem
ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver: 
0x112848, name: huaweimodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x1127d0, name: iceramodem
ofonod[5126]: 
src/radio-settings.c:ofono_radio_settings_driver_unregister() driver: 
0x1127fc, name: iceramodem
ofonod[5126]: 
src/radio-settings.c:ofono_radio_settings_driver_unregister() driver: 
0x11277c, name: ztemodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x112738, name: swmodem
ofonod[5126]: 
src/radio-settings.c:ofono_radio_settings_driver_unregister() driver: 
0x1126f4, name: nwmodem
ofonod[5126]: src/sim-auth.c:ofono_sim_auth_driver_unregister() driver: 
0x112698, name: atmodem
ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112520, 
name: atmodem
ofonod[5126]: src/sim.c:ofono_sim_driver_unregister() driver: 0x112488, 
name: atmodem
ofonod[5126]: src/sim.c:ofono_sim_driver_unregister() driver: 0x1124d0, 
name: atmodem-noef
ofonod[5126]: src/sms.c:ofono_sms_driver_unregister() driver: 0x1122e0, 
name: atmodem
ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver: 
0x112548, name: atmodem
ofonod[5126]: src/phonebook.c:ofono_phonebook_driver_unregister() 
driver: 0x1125ec, name: atmodem
ofonod[5126]: 
src/call-settings.c:ofono_call_settings_driver_unregister() driver: 
0x112270, name: atmodem
ofonod[5126]: src/call-meter.c:ofono_call_meter_driver_unregister() 
driver: 0x112364, name: atmodem
ofonod[5126]: 
src/call-forwarding.c:ofono_call_forwarding_driver_unregister() driver: 
0x112324, name: atmodem
ofonod[5126]: src/call-barring.c:ofono_call_barring_driver_unregister() 
driver: 0x1125bc, name: atmodem
ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver: 
0x1123d0, name: atmodem
ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver: 
0x112604, name: atmodem
ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister() 
driver: 0x11256c, name: atmodem
ofonod[5126]: src/cbs.c:ofono_cbs_driver_unregister() driver: 0x112308, 
name: atmodem
ofonod[5126]: src/call-volume.c:ofono_call_volume_driver_unregister() 
driver: 0x112630, name: atmodem
ofonod[5126]: src/gprs.c:ofono_gprs_driver_unregister() driver: 
0x112660, name: atmodem
ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver: 
0x112674, name: atmodem
ofonod[5126]: src/gnss.c:ofono_gnss_driver_unregister() driver: 
0x1126b8, name: atmodem
ofonod[5126]: src/modem.c:modem_unregister() 0xe18f88
ofonod[5126]: src/modem.c:modem_change_state() old state: 3, new state: 0
ofonod[5126]: src/modem.c:flush_atoms()
ofonod[5126]: src/sim.c:ofono_sim_remove_spn_watch() 0xe17418
ofonod[5126]: src/network.c:netreg_remove() atom: 0xe17af8
ofonod[5126]: src/voicecall.c:voicecall_remove() atom: 0xe17c58
ofonod[5126]: src/gprs.c:gprs_context_unregister() 0xe17cd8, 0xe17a30
ofonod[5126]: src/gprs.c:gprs_context_remove() atom: 0xe17cf8
ofonod[5126]: drivers/atmodem/gprs-context.c:at_gprs_context_remove()
ofonod[5126]: src/gprs.c:gprs_unregister() 0xe17a30
ofonod[5126]: src/gprs.c:gprs_remove() atom: 0xe17b48
ofonod[5126]: plugins/push-notification.c:push_notification_cleanup() 
0xe1adb0
ofonod[5126]: plugins/smart-messaging.c:smart_messaging_cleanup() 0xe1ad80
ofonod[5126]: src/sms.c:sms_remove() atom: 0xe175a0
Floating point exception (core dumped)

>
>> ---
>>   plugins/push-notification.c | 26 +++++++++++++++++++++++---
>>   1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/plugins/push-notification.c b/plugins/push-notification.c
>> index ff388d9..ecf100f 100644
>> --- a/plugins/push-notification.c
>> +++ b/plugins/push-notification.c
>> @@ -45,6 +45,7 @@
>>   #define WAP_PUSH_DST_PORT 2948
>>
>>   static unsigned int modemwatch_id;
>> +static GHashTable *sms_watches = NULL;
>>
>>   struct push_notification {
>>       struct ofono_modem *modem;
>> @@ -164,6 +165,16 @@ static void push_notification_cleanup(gpointer 
>> user)
>>       ofono_modem_remove_interface(pn->modem, 
>> PUSH_NOTIFICATION_INTERFACE);
>>   }
>>
>> +static gboolean atom_watch_remove(gpointer key, gpointer value,
>> +                    gpointer user_data)
>> +{
>> +    struct ofono_modem *modem = key;
>> +
>> +    __ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
>> +
>> +    return TRUE;
>> +}
>> +
>>   static void sms_watch(struct ofono_atom *atom,
>>                   enum ofono_atom_watch_condition cond,
>>                   void *data)
>> @@ -197,18 +208,22 @@ static void sms_watch(struct ofono_atom *atom,
>>   static void modem_watch(struct ofono_modem *modem, gboolean added, 
>> void *user)
>>   {
>>       struct push_notification *pn;
>> +    int sms;
>>       DBG("modem: %p, added: %d", modem, added);
>>
>> -    if (added == FALSE)
>> +    if (added == FALSE) {
>> +        g_hash_table_remove(sms_watches, modem);
>>           return;
>> +    }
>
> This has no effect.  The atom_watches for that particular modem have 
> already been freed by the time the modem watch has been called in 
> call_modemwatches().  See modem_unregister for details.
Depends on which exit is called first, for at least some plugins the 
exit can be called before modem_unregister. So the plugin is already 
cleaned up when its atom watch is being called. This is the root of the 
SIGFPE in glib.
This just removes the entry in the local hash table, as the plugin exit 
function will now unregister the watches if there are any active during 
exit.
>
>>
>>       pn = g_try_new0(struct push_notification, 1);
>>       if (pn == NULL)
>>           return;
>>
>>       pn->modem = modem;
>> -    __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
>> -                    sms_watch, pn, g_free);
>> +    sms = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
>> +                        sms_watch, pn, g_free);
>> +    g_hash_table_insert(sms_watches, modem, GUINT_TO_POINTER(sms));
>>   }
>>
>>   static void call_modemwatch(struct ofono_modem *modem, void *user)
>> @@ -220,6 +235,8 @@ static int push_notification_init(void)
>>   {
>>       DBG("");
>>
>> +    sms_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
>> +
>>       modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
>>
>>       __ofono_modem_foreach(call_modemwatch, NULL);
>> @@ -232,6 +249,9 @@ static void push_notification_exit(void)
>>       DBG("");
>>
>>       __ofono_modemwatch_remove(modemwatch_id);
>> +
>> +    g_hash_table_foreach_remove(sms_watches, atom_watch_remove, NULL);
>> +    g_hash_table_destroy(sms_watches);
>
> atom watches are already being removed by the virtue of modems being 
> unregistered.
>
>>   }
>>
>>   OFONO_PLUGIN_DEFINE(push_notification, "Push Notification Plugin", 
>> VERSION,
>>
>
> Regards,
> -Denis
I hope this clarifies the reason for this patch. I deeply apologize for 
not including this information with the patch submission.

Best regards // John Ernberg

Re: [PATCH 4/5] push notification: track and clean up atom watches

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 17558 bytes --]

Hi John,

On 04/25/2016 02:02 AM, John Ernberg wrote:
> Hi Denis,
>
> On 04/22/2016 09:43 PM, Denis Kenzior wrote:
>> Hi John,
>>
>> On 04/22/2016 08:07 AM, John Ernberg wrote:
>>> From: John Ernberg <john.ernberg@actia.se>
>>>
>>> Prevents glib from causing SIGFPE during certain circumstances of modem
>>> removal.
>>
>> Do you have a stack trace handy?
> I have one for the bluez5 hfp plugin which was recreated by stopping
> oFono, the rest of the changes were added as a sort of safeguard.
>
> Stack trace:
>
> (gdb)
> (gdb) bt
> #0  0x76d88748 in raise (sig=8)
>       at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
> #1  0x76e8bcac in __aeabi_ldiv0 ()
>       at
> /opt/yocto/build/tmp/work-shared/gcc-4.8.2-r0/gcc-4.8.2/libgcc/config/arm/lib1funcs.S:1331
> #2  0x76e0e1b4 in g_hash_table_lookup_node (hash_return=<synthetic
> pointer>,
>       key=0xe17418, hash_table=0xe10e38)
>       at /usr/src/debug/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/ghash.c:371
> #3  g_hash_table_lookup (hash_table=0xe10e38, key=key(a)entry=0xe17418)
>       at /usr/src/debug/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/ghash.c:1076
> #4  0x0008ccb8 in sim_watch (atom=<optimized out>, cond=<optimized out>,
>       data=0xe18f88) at plugins/hfp_ag_bluez5.c:260
> #5  0x000910d0 in call_watches (atom=atom(a)entry=0xe17540,
>       cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at
> src/modem.c:261
> #6  0x00091de0 in __ofono_atom_unregister (atom=atom(a)entry=0xe17540)
>       at src/modem.c:281
> #7  0x00091e54 in flush_atoms (
>       new_state=new_state(a)entry=MODEM_STATE_POWER_OFF, modem=0xe18f88)
>       at src/modem.c:430
> #8  0x00091f60 in modem_change_state (modem=0xe18f88,
>       new_state=MODEM_STATE_POWER_OFF) at src/modem.c:511
> #9  0x000921c8 in set_powered (modem=modem(a)entry=0xe18f88,
>       powered=powered(a)entry=0) at src/modem.c:882
> #10 0x00092990 in modem_unregister (modem=0xe18f88) at src/modem.c:2040
> #11 0x00093cec in ofono_modem_driver_unregister (d=0x112168)
>       at src/modem.c:2176

What calls this?  The corresponding exit function in question is not 
present in the stack trace.

> #12 0x00091040 in __ofono_plugin_cleanup () at src/plugin.c:197
> #13 0x00014e24 in main (argc=85540, argv=0x7ee60d44) at src/main.c:255
>
> oFono debug log:
>
> ofonod[5126]: src/plugin.c:__ofono_plugin_cleanup()
> ofonod[5126]: plugins/push-notification.c:push_notification_exit()
> ofonod[5126]: plugins/smart-messaging.c:smart_messaging_exit()
> ofonod[5126]: plugins/bluez5.c:bt_unregister_profile() Bluetooth:
> Unregistering profile /bluetooth/profile/dun_gw
> ofonod[5126]: plugins/bluez5.c:bt_unregister_profile() Bluetooth:
> Unregistering profile /bluetooth/profile/hfp_hf
> ofonod[5126]:
> src/handsfree-audio.c:ofono_handsfree_card_driver_unregister() driver:
> 0x113994
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1139a4, name: hfp
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x11391c, name: ublox
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1138b4, name: quectel
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113854, name: he910
> ofonod[5126]:
> src/private-network.c:ofono_private_network_driver_unregister() driver:
> 0x113828, name: ConnMan Private Network
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1137d0, name: sim900
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113778, name: samsung
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113720, name: speedupcdma
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1136c0, name: speedup
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113668, name: alcatel
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113600, name: icera
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1135a8, name: linktop
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113550, name: nokiacdma
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1134f8, name: nokia
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1134a0, name: tc65
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113408, name: ste
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1133a0, name: ifx
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113348, name: palmpre
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1132e8, name: novatel
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113290, name: sierra
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113208, name: huawei
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1131b0, name: zte
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113140, name: hso
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x1130e0, name: mbm
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113080, name: calypso
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x113028, name: wavecom
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x112fd0, name: g1
> ofonod[5126]:
> src/cdma-voicecall.c:ofono_cdma_voicecall_driver_unregister() driver:
> 0x112f78, name: cdmamodem
> ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver:
> 0x112fa0, name: cdmamodem
> ofonod[5126]: src/cdma-connman.c:ofono_cdma_connman_driver_unregister()
> driver: 0x112fbc, name: cdmamodem
> ofonod[5126]: src/ctm.c:ofono_ctm_driver_unregister() driver: 0x112e88,
> name: phonesim
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x112e9c, name: phonesim
> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
> 0x112eb8, name: phonesim
> ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver:
> 0x112e74, name: speedupmodem
> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
> driver: 0x112d38, name: hfpmodem
> ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver:
> 0x112ddc, name: hfpmodem
> ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver:
> 0x112d90, name: hfpmodem
> ofonod[5126]: src/call-volume.c:ofono_call_volume_driver_unregister()
> driver: 0x112dc4, name: hfpmodem
> ofonod[5126]: src/handsfree.c:ofono_handsfree_driver_unregister()
> driver: 0x112e08, name: hfpmodem
> ofonod[5126]: src/siri.c:ofono_siri_driver_unregister() driver:
> 0x112e3c, name: hfpmodem
> ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver:
> 0x112cb8, name: dunmodem
> ofonod[5126]: src/gprs.c:ofono_gprs_driver_unregister() driver:
> 0x112cdc, name: dunmodem
> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
> driver: 0x112bec, name: stemodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x112c74, name: stemodem
> ofonod[5126]:
> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
> 0x112c3c, name: stemodem
> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112b98,
> name: ifxmodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x112b6c, name: ifxmodem
> ofonod[5126]:
> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
> 0x112b38, name: ifxmodem
> ofonod[5126]:
> src/audio-settings.c:ofono_audio_settings_driver_unregister() driver:
> 0x112b24, name: ifxmodem
> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
> driver: 0x112acc, name: ifxmodem
> ofonod[5126]: src/ctm.c:ofono_ctm_driver_unregister() driver: 0x112bb8,
> name: ifxmodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x112a5c, name: hsomodem
> ofonod[5126]:
> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
> 0x112a80, name: hsomodem
> ofonod[5126]:
> src/location-reporting.c:ofono_location_reporting_driver_unregister()
> driver: 0x112a1c, name: mbmmodem
> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x1129fc,
> name: mbmmodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x1129d8, name: mbmmodem
> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112990,
> name: calypsomodem
> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
> driver: 0x112940, name: calypsomodem
> ofonod[5126]: src/cdma-netreg.c:ofono_cdma_netreg_driver_unregister()
> driver: 0x112910, name: huaweimodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x1128c0, name: huaweimodem
> ofonod[5126]:
> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
> 0x1128e4, name: huaweimodem
> ofonod[5126]:
> src/audio-settings.c:ofono_audio_settings_driver_unregister() driver:
> 0x1128ac, name: huaweimodem
> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
> driver: 0x11285c, name: huaweimodem
> ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver:
> 0x112848, name: huaweimodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x1127d0, name: iceramodem
> ofonod[5126]:
> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
> 0x1127fc, name: iceramodem
> ofonod[5126]:
> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
> 0x11277c, name: ztemodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x112738, name: swmodem
> ofonod[5126]:
> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
> 0x1126f4, name: nwmodem
> ofonod[5126]: src/sim-auth.c:ofono_sim_auth_driver_unregister() driver:
> 0x112698, name: atmodem
> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112520,
> name: atmodem
> ofonod[5126]: src/sim.c:ofono_sim_driver_unregister() driver: 0x112488,
> name: atmodem
> ofonod[5126]: src/sim.c:ofono_sim_driver_unregister() driver: 0x1124d0,
> name: atmodem-noef
> ofonod[5126]: src/sms.c:ofono_sms_driver_unregister() driver: 0x1122e0,
> name: atmodem
> ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver:
> 0x112548, name: atmodem
> ofonod[5126]: src/phonebook.c:ofono_phonebook_driver_unregister()
> driver: 0x1125ec, name: atmodem
> ofonod[5126]:
> src/call-settings.c:ofono_call_settings_driver_unregister() driver:
> 0x112270, name: atmodem
> ofonod[5126]: src/call-meter.c:ofono_call_meter_driver_unregister()
> driver: 0x112364, name: atmodem
> ofonod[5126]:
> src/call-forwarding.c:ofono_call_forwarding_driver_unregister() driver:
> 0x112324, name: atmodem
> ofonod[5126]: src/call-barring.c:ofono_call_barring_driver_unregister()
> driver: 0x1125bc, name: atmodem
> ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver:
> 0x1123d0, name: atmodem
> ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver:
> 0x112604, name: atmodem
> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
> driver: 0x11256c, name: atmodem
> ofonod[5126]: src/cbs.c:ofono_cbs_driver_unregister() driver: 0x112308,
> name: atmodem
> ofonod[5126]: src/call-volume.c:ofono_call_volume_driver_unregister()
> driver: 0x112630, name: atmodem
> ofonod[5126]: src/gprs.c:ofono_gprs_driver_unregister() driver:
> 0x112660, name: atmodem
> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
> 0x112674, name: atmodem
> ofonod[5126]: src/gnss.c:ofono_gnss_driver_unregister() driver:
> 0x1126b8, name: atmodem
> ofonod[5126]: src/modem.c:modem_unregister() 0xe18f88
> ofonod[5126]: src/modem.c:modem_change_state() old state: 3, new state: 0

Are you running a modified version of oFono by any chance?  The upstream 
version handles SIGINT and SIGTERM by calling __ofono_modem_shutdown. 
Which in turn flushes all atoms and all the various watches are cleaned up.

Only after all the modems are shut down do we exit the event loop and 
__ofono_plugin_cleanup is called.  The fact that you still have a modem 
in state MODEM_STATE_ONLINE here is a bit puzzling.

However, even if I take out that logic and force the debug log to look 
like yours, the only problems I detect are in the hfp_ag_bluez5 plugin 
and examples/emulator.c plugin.

> ofonod[5126]: src/modem.c:flush_atoms()
> ofonod[5126]: src/sim.c:ofono_sim_remove_spn_watch() 0xe17418
> ofonod[5126]: src/network.c:netreg_remove() atom: 0xe17af8
> ofonod[5126]: src/voicecall.c:voicecall_remove() atom: 0xe17c58
> ofonod[5126]: src/gprs.c:gprs_context_unregister() 0xe17cd8, 0xe17a30
> ofonod[5126]: src/gprs.c:gprs_context_remove() atom: 0xe17cf8
> ofonod[5126]: drivers/atmodem/gprs-context.c:at_gprs_context_remove()
> ofonod[5126]: src/gprs.c:gprs_unregister() 0xe17a30
> ofonod[5126]: src/gprs.c:gprs_remove() atom: 0xe17b48
> ofonod[5126]: plugins/push-notification.c:push_notification_cleanup()
> 0xe1adb0
> ofonod[5126]: plugins/smart-messaging.c:smart_messaging_cleanup() 0xe1ad80
> ofonod[5126]: src/sms.c:sms_remove() atom: 0xe175a0
> Floating point exception (core dumped)
>
>>
>>> ---
>>>    plugins/push-notification.c | 26 +++++++++++++++++++++++---
>>>    1 file changed, 23 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/plugins/push-notification.c b/plugins/push-notification.c
>>> index ff388d9..ecf100f 100644
>>> --- a/plugins/push-notification.c
>>> +++ b/plugins/push-notification.c
>>> @@ -45,6 +45,7 @@
>>>    #define WAP_PUSH_DST_PORT 2948
>>>
>>>    static unsigned int modemwatch_id;
>>> +static GHashTable *sms_watches = NULL;
>>>
>>>    struct push_notification {
>>>        struct ofono_modem *modem;
>>> @@ -164,6 +165,16 @@ static void push_notification_cleanup(gpointer
>>> user)
>>>        ofono_modem_remove_interface(pn->modem,
>>> PUSH_NOTIFICATION_INTERFACE);
>>>    }
>>>
>>> +static gboolean atom_watch_remove(gpointer key, gpointer value,
>>> +                    gpointer user_data)
>>> +{
>>> +    struct ofono_modem *modem = key;
>>> +
>>> +    __ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
>>> +
>>> +    return TRUE;
>>> +}
>>> +
>>>    static void sms_watch(struct ofono_atom *atom,
>>>                    enum ofono_atom_watch_condition cond,
>>>                    void *data)
>>> @@ -197,18 +208,22 @@ static void sms_watch(struct ofono_atom *atom,
>>>    static void modem_watch(struct ofono_modem *modem, gboolean added,
>>> void *user)
>>>    {
>>>        struct push_notification *pn;
>>> +    int sms;
>>>        DBG("modem: %p, added: %d", modem, added);
>>>
>>> -    if (added == FALSE)
>>> +    if (added == FALSE) {
>>> +        g_hash_table_remove(sms_watches, modem);
>>>            return;
>>> +    }
>>
>> This has no effect.  The atom_watches for that particular modem have
>> already been freed by the time the modem watch has been called in
>> call_modemwatches().  See modem_unregister for details.
> Depends on which exit is called first, for at least some plugins the
> exit can be called before modem_unregister. So the plugin is already
> cleaned up when its atom watch is being called. This is the root of the
> SIGFPE in glib.
> This just removes the entry in the local hash table, as the plugin exit
> function will now unregister the watches if there are any active during
> exit.

I still don't see how it matters in this case.  Even if 
push_notification plugin is cleaned up first, all it will do is remove 
the modemwatch.  All actual watches created will be cleaned up when the 
modem is removed.

I suspect the issue is isolated to hfp_ag_bluez5 plugin only.

>>
>>>
>>>        pn = g_try_new0(struct push_notification, 1);
>>>        if (pn == NULL)
>>>            return;
>>>
>>>        pn->modem = modem;
>>> -    __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
>>> -                    sms_watch, pn, g_free);
>>> +    sms = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
>>> +                        sms_watch, pn, g_free);
>>> +    g_hash_table_insert(sms_watches, modem, GUINT_TO_POINTER(sms));
>>>    }
>>>
>>>    static void call_modemwatch(struct ofono_modem *modem, void *user)
>>> @@ -220,6 +235,8 @@ static int push_notification_init(void)
>>>    {
>>>        DBG("");
>>>
>>> +    sms_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
>>> +
>>>        modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
>>>
>>>        __ofono_modem_foreach(call_modemwatch, NULL);
>>> @@ -232,6 +249,9 @@ static void push_notification_exit(void)
>>>        DBG("");
>>>
>>>        __ofono_modemwatch_remove(modemwatch_id);
>>> +
>>> +    g_hash_table_foreach_remove(sms_watches, atom_watch_remove, NULL);
>>> +    g_hash_table_destroy(sms_watches);
>>
>> atom watches are already being removed by the virtue of modems being
>> unregistered.
>>
>>>    }
>>>
>>>    OFONO_PLUGIN_DEFINE(push_notification, "Push Notification Plugin",
>>> VERSION,
>>>
>>
>> Regards,
>> -Denis
> I hope this clarifies the reason for this patch. I deeply apologize for
> not including this information with the patch submission.
>
> Best regards // John Ernberg
>

Regards,
-Denis

Re: [PATCH 4/5] push notification: track and clean up atom watches

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 18803 bytes --]

Hi Denis,

On 04/25/2016 08:56 PM, Denis Kenzior wrote:
> Hi John,
>
> On 04/25/2016 02:02 AM, John Ernberg wrote:
>> Hi Denis,
>>
>> On 04/22/2016 09:43 PM, Denis Kenzior wrote:
>>> Hi John,
>>>
>>> On 04/22/2016 08:07 AM, John Ernberg wrote:
>>>> From: John Ernberg <john.ernberg@actia.se>
>>>>
>>>> Prevents glib from causing SIGFPE during certain circumstances of 
>>>> modem
>>>> removal.
>>>
>>> Do you have a stack trace handy?
>> I have one for the bluez5 hfp plugin which was recreated by stopping
>> oFono, the rest of the changes were added as a sort of safeguard.
>>
>> Stack trace:
>>
>> (gdb)
>> (gdb) bt
>> #0  0x76d88748 in raise (sig=8)
>>       at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:37
>> #1  0x76e8bcac in __aeabi_ldiv0 ()
>>       at
>> /opt/yocto/build/tmp/work-shared/gcc-4.8.2-r0/gcc-4.8.2/libgcc/config/arm/lib1funcs.S:1331 
>>
>> #2  0x76e0e1b4 in g_hash_table_lookup_node (hash_return=<synthetic
>> pointer>,
>>       key=0xe17418, hash_table=0xe10e38)
>>       at 
>> /usr/src/debug/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/ghash.c:371
>> #3  g_hash_table_lookup (hash_table=0xe10e38, key=key(a)entry=0xe17418)
>>       at 
>> /usr/src/debug/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/ghash.c:1076
>> #4  0x0008ccb8 in sim_watch (atom=<optimized out>, cond=<optimized out>,
>>       data=0xe18f88) at plugins/hfp_ag_bluez5.c:260
>> #5  0x000910d0 in call_watches (atom=atom(a)entry=0xe17540,
>>       cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at
>> src/modem.c:261
>> #6  0x00091de0 in __ofono_atom_unregister (atom=atom(a)entry=0xe17540)
>>       at src/modem.c:281
>> #7  0x00091e54 in flush_atoms (
>>       new_state=new_state(a)entry=MODEM_STATE_POWER_OFF, modem=0xe18f88)
>>       at src/modem.c:430
>> #8  0x00091f60 in modem_change_state (modem=0xe18f88,
>>       new_state=MODEM_STATE_POWER_OFF) at src/modem.c:511
>> #9  0x000921c8 in set_powered (modem=modem(a)entry=0xe18f88,
>>       powered=powered(a)entry=0) at src/modem.c:882
>> #10 0x00092990 in modem_unregister (modem=0xe18f88) at src/modem.c:2040
>> #11 0x00093cec in ofono_modem_driver_unregister (d=0x112168)
>>       at src/modem.c:2176
>
> What calls this?  The corresponding exit function in question is not 
> present in the stack trace.
The modem plugins call it during their exit. Since the plugins are all 
calling exit the drivers gets unregistered.
>
>> #12 0x00091040 in __ofono_plugin_cleanup () at src/plugin.c:197
>> #13 0x00014e24 in main (argc=85540, argv=0x7ee60d44) at src/main.c:255
>>
>> oFono debug log:
>>
>> ofonod[5126]: src/plugin.c:__ofono_plugin_cleanup()
>> ofonod[5126]: plugins/push-notification.c:push_notification_exit()
>> ofonod[5126]: plugins/smart-messaging.c:smart_messaging_exit()
>> ofonod[5126]: plugins/bluez5.c:bt_unregister_profile() Bluetooth:
>> Unregistering profile /bluetooth/profile/dun_gw
>> ofonod[5126]: plugins/bluez5.c:bt_unregister_profile() Bluetooth:
>> Unregistering profile /bluetooth/profile/hfp_hf
>> ofonod[5126]:
>> src/handsfree-audio.c:ofono_handsfree_card_driver_unregister() driver:
>> 0x113994
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1139a4, name: hfp
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x11391c, name: ublox
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1138b4, name: quectel
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113854, name: he910
>> ofonod[5126]:
>> src/private-network.c:ofono_private_network_driver_unregister() driver:
>> 0x113828, name: ConnMan Private Network
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1137d0, name: sim900
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113778, name: samsung
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113720, name: speedupcdma
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1136c0, name: speedup
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113668, name: alcatel
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113600, name: icera
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1135a8, name: linktop
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113550, name: nokiacdma
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1134f8, name: nokia
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1134a0, name: tc65
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113408, name: ste
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1133a0, name: ifx
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113348, name: palmpre
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1132e8, name: novatel
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113290, name: sierra
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113208, name: huawei
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1131b0, name: zte
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113140, name: hso
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x1130e0, name: mbm
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113080, name: calypso
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x113028, name: wavecom
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x112fd0, name: g1
>> ofonod[5126]:
>> src/cdma-voicecall.c:ofono_cdma_voicecall_driver_unregister() driver:
>> 0x112f78, name: cdmamodem
>> ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver:
>> 0x112fa0, name: cdmamodem
>> ofonod[5126]: src/cdma-connman.c:ofono_cdma_connman_driver_unregister()
>> driver: 0x112fbc, name: cdmamodem
>> ofonod[5126]: src/ctm.c:ofono_ctm_driver_unregister() driver: 0x112e88,
>> name: phonesim
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x112e9c, name: phonesim
>> ofonod[5126]: src/modem.c:ofono_modem_driver_unregister() driver:
>> 0x112eb8, name: phonesim
>> ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver:
>> 0x112e74, name: speedupmodem
>> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
>> driver: 0x112d38, name: hfpmodem
>> ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver:
>> 0x112ddc, name: hfpmodem
>> ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver:
>> 0x112d90, name: hfpmodem
>> ofonod[5126]: src/call-volume.c:ofono_call_volume_driver_unregister()
>> driver: 0x112dc4, name: hfpmodem
>> ofonod[5126]: src/handsfree.c:ofono_handsfree_driver_unregister()
>> driver: 0x112e08, name: hfpmodem
>> ofonod[5126]: src/siri.c:ofono_siri_driver_unregister() driver:
>> 0x112e3c, name: hfpmodem
>> ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver:
>> 0x112cb8, name: dunmodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_driver_unregister() driver:
>> 0x112cdc, name: dunmodem
>> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
>> driver: 0x112bec, name: stemodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x112c74, name: stemodem
>> ofonod[5126]:
>> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
>> 0x112c3c, name: stemodem
>> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112b98,
>> name: ifxmodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x112b6c, name: ifxmodem
>> ofonod[5126]:
>> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
>> 0x112b38, name: ifxmodem
>> ofonod[5126]:
>> src/audio-settings.c:ofono_audio_settings_driver_unregister() driver:
>> 0x112b24, name: ifxmodem
>> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
>> driver: 0x112acc, name: ifxmodem
>> ofonod[5126]: src/ctm.c:ofono_ctm_driver_unregister() driver: 0x112bb8,
>> name: ifxmodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x112a5c, name: hsomodem
>> ofonod[5126]:
>> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
>> 0x112a80, name: hsomodem
>> ofonod[5126]:
>> src/location-reporting.c:ofono_location_reporting_driver_unregister()
>> driver: 0x112a1c, name: mbmmodem
>> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x1129fc,
>> name: mbmmodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x1129d8, name: mbmmodem
>> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112990,
>> name: calypsomodem
>> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
>> driver: 0x112940, name: calypsomodem
>> ofonod[5126]: src/cdma-netreg.c:ofono_cdma_netreg_driver_unregister()
>> driver: 0x112910, name: huaweimodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x1128c0, name: huaweimodem
>> ofonod[5126]:
>> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
>> 0x1128e4, name: huaweimodem
>> ofonod[5126]:
>> src/audio-settings.c:ofono_audio_settings_driver_unregister() driver:
>> 0x1128ac, name: huaweimodem
>> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
>> driver: 0x11285c, name: huaweimodem
>> ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver:
>> 0x112848, name: huaweimodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x1127d0, name: iceramodem
>> ofonod[5126]:
>> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
>> 0x1127fc, name: iceramodem
>> ofonod[5126]:
>> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
>> 0x11277c, name: ztemodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x112738, name: swmodem
>> ofonod[5126]:
>> src/radio-settings.c:ofono_radio_settings_driver_unregister() driver:
>> 0x1126f4, name: nwmodem
>> ofonod[5126]: src/sim-auth.c:ofono_sim_auth_driver_unregister() driver:
>> 0x112698, name: atmodem
>> ofonod[5126]: src/stk.c:ofono_stk_driver_unregister() driver: 0x112520,
>> name: atmodem
>> ofonod[5126]: src/sim.c:ofono_sim_driver_unregister() driver: 0x112488,
>> name: atmodem
>> ofonod[5126]: src/sim.c:ofono_sim_driver_unregister() driver: 0x1124d0,
>> name: atmodem-noef
>> ofonod[5126]: src/sms.c:ofono_sms_driver_unregister() driver: 0x1122e0,
>> name: atmodem
>> ofonod[5126]: src/ussd.c:ofono_ussd_driver_unregister() driver:
>> 0x112548, name: atmodem
>> ofonod[5126]: src/phonebook.c:ofono_phonebook_driver_unregister()
>> driver: 0x1125ec, name: atmodem
>> ofonod[5126]:
>> src/call-settings.c:ofono_call_settings_driver_unregister() driver:
>> 0x112270, name: atmodem
>> ofonod[5126]: src/call-meter.c:ofono_call_meter_driver_unregister()
>> driver: 0x112364, name: atmodem
>> ofonod[5126]:
>> src/call-forwarding.c:ofono_call_forwarding_driver_unregister() driver:
>> 0x112324, name: atmodem
>> ofonod[5126]: src/call-barring.c:ofono_call_barring_driver_unregister()
>> driver: 0x1125bc, name: atmodem
>> ofonod[5126]: src/network.c:ofono_netreg_driver_unregister() driver:
>> 0x1123d0, name: atmodem
>> ofonod[5126]: src/modem.c:ofono_devinfo_driver_unregister() driver:
>> 0x112604, name: atmodem
>> ofonod[5126]: src/voicecall.c:ofono_voicecall_driver_unregister()
>> driver: 0x11256c, name: atmodem
>> ofonod[5126]: src/cbs.c:ofono_cbs_driver_unregister() driver: 0x112308,
>> name: atmodem
>> ofonod[5126]: src/call-volume.c:ofono_call_volume_driver_unregister()
>> driver: 0x112630, name: atmodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_driver_unregister() driver:
>> 0x112660, name: atmodem
>> ofonod[5126]: src/gprs.c:ofono_gprs_context_driver_unregister() driver:
>> 0x112674, name: atmodem
>> ofonod[5126]: src/gnss.c:ofono_gnss_driver_unregister() driver:
>> 0x1126b8, name: atmodem
>> ofonod[5126]: src/modem.c:modem_unregister() 0xe18f88
>> ofonod[5126]: src/modem.c:modem_change_state() old state: 3, new 
>> state: 0
>
> Are you running a modified version of oFono by any chance?  The 
> upstream version handles SIGINT and SIGTERM by calling 
> __ofono_modem_shutdown. Which in turn flushes all atoms and all the 
> various watches are cleaned up.
In the embedded system where I run oFono I have not figured out how to 
get the stack tracing implemented in src/log.c working so that is disabled.
I have not done any changes to src/main.c or src/log.c except disabling 
the internal stack trace.
>
> Only after all the modems are shut down do we exit the event loop and 
> __ofono_plugin_cleanup is called.  The fact that you still have a 
> modem in state MODEM_STATE_ONLINE here is a bit puzzling.
>
> However, even if I take out that logic and force the debug log to look 
> like yours, the only problems I detect are in the hfp_ag_bluez5 plugin 
> and examples/emulator.c plugin.
Hm, if it doesn't happen in upstream this is also probably PEBKAC on my 
part somehow.
>
>> ofonod[5126]: src/modem.c:flush_atoms()
>> ofonod[5126]: src/sim.c:ofono_sim_remove_spn_watch() 0xe17418
>> ofonod[5126]: src/network.c:netreg_remove() atom: 0xe17af8
>> ofonod[5126]: src/voicecall.c:voicecall_remove() atom: 0xe17c58
>> ofonod[5126]: src/gprs.c:gprs_context_unregister() 0xe17cd8, 0xe17a30
>> ofonod[5126]: src/gprs.c:gprs_context_remove() atom: 0xe17cf8
>> ofonod[5126]: drivers/atmodem/gprs-context.c:at_gprs_context_remove()
>> ofonod[5126]: src/gprs.c:gprs_unregister() 0xe17a30
>> ofonod[5126]: src/gprs.c:gprs_remove() atom: 0xe17b48
>> ofonod[5126]: plugins/push-notification.c:push_notification_cleanup()
>> 0xe1adb0
>> ofonod[5126]: plugins/smart-messaging.c:smart_messaging_cleanup() 
>> 0xe1ad80
>> ofonod[5126]: src/sms.c:sms_remove() atom: 0xe175a0
>> Floating point exception (core dumped)
>>
>>>
>>>> ---
>>>>    plugins/push-notification.c | 26 +++++++++++++++++++++++---
>>>>    1 file changed, 23 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/plugins/push-notification.c b/plugins/push-notification.c
>>>> index ff388d9..ecf100f 100644
>>>> --- a/plugins/push-notification.c
>>>> +++ b/plugins/push-notification.c
>>>> @@ -45,6 +45,7 @@
>>>>    #define WAP_PUSH_DST_PORT 2948
>>>>
>>>>    static unsigned int modemwatch_id;
>>>> +static GHashTable *sms_watches = NULL;
>>>>
>>>>    struct push_notification {
>>>>        struct ofono_modem *modem;
>>>> @@ -164,6 +165,16 @@ static void push_notification_cleanup(gpointer
>>>> user)
>>>>        ofono_modem_remove_interface(pn->modem,
>>>> PUSH_NOTIFICATION_INTERFACE);
>>>>    }
>>>>
>>>> +static gboolean atom_watch_remove(gpointer key, gpointer value,
>>>> +                    gpointer user_data)
>>>> +{
>>>> +    struct ofono_modem *modem = key;
>>>> +
>>>> +    __ofono_modem_remove_atom_watch(modem, GPOINTER_TO_UINT(value));
>>>> +
>>>> +    return TRUE;
>>>> +}
>>>> +
>>>>    static void sms_watch(struct ofono_atom *atom,
>>>>                    enum ofono_atom_watch_condition cond,
>>>>                    void *data)
>>>> @@ -197,18 +208,22 @@ static void sms_watch(struct ofono_atom *atom,
>>>>    static void modem_watch(struct ofono_modem *modem, gboolean added,
>>>> void *user)
>>>>    {
>>>>        struct push_notification *pn;
>>>> +    int sms;
>>>>        DBG("modem: %p, added: %d", modem, added);
>>>>
>>>> -    if (added == FALSE)
>>>> +    if (added == FALSE) {
>>>> +        g_hash_table_remove(sms_watches, modem);
>>>>            return;
>>>> +    }
>>>
>>> This has no effect.  The atom_watches for that particular modem have
>>> already been freed by the time the modem watch has been called in
>>> call_modemwatches().  See modem_unregister for details.
>> Depends on which exit is called first, for at least some plugins the
>> exit can be called before modem_unregister. So the plugin is already
>> cleaned up when its atom watch is being called. This is the root of the
>> SIGFPE in glib.
>> This just removes the entry in the local hash table, as the plugin exit
>> function will now unregister the watches if there are any active during
>> exit.
>
> I still don't see how it matters in this case.  Even if 
> push_notification plugin is cleaned up first, all it will do is remove 
> the modemwatch.  All actual watches created will be cleaned up when 
> the modem is removed.
>
> I suspect the issue is isolated to hfp_ag_bluez5 plugin only.
>
>>>
>>>>
>>>>        pn = g_try_new0(struct push_notification, 1);
>>>>        if (pn == NULL)
>>>>            return;
>>>>
>>>>        pn->modem = modem;
>>>> -    __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
>>>> -                    sms_watch, pn, g_free);
>>>> +    sms = __ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SMS,
>>>> +                        sms_watch, pn, g_free);
>>>> +    g_hash_table_insert(sms_watches, modem, GUINT_TO_POINTER(sms));
>>>>    }
>>>>
>>>>    static void call_modemwatch(struct ofono_modem *modem, void *user)
>>>> @@ -220,6 +235,8 @@ static int push_notification_init(void)
>>>>    {
>>>>        DBG("");
>>>>
>>>> +    sms_watches = g_hash_table_new(g_direct_hash, g_direct_equal);
>>>> +
>>>>        modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, 
>>>> NULL);
>>>>
>>>>        __ofono_modem_foreach(call_modemwatch, NULL);
>>>> @@ -232,6 +249,9 @@ static void push_notification_exit(void)
>>>>        DBG("");
>>>>
>>>>        __ofono_modemwatch_remove(modemwatch_id);
>>>> +
>>>> +    g_hash_table_foreach_remove(sms_watches, atom_watch_remove, 
>>>> NULL);
>>>> +    g_hash_table_destroy(sms_watches);
>>>
>>> atom watches are already being removed by the virtue of modems being
>>> unregistered.
>>>
>>>>    }
>>>>
>>>>    OFONO_PLUGIN_DEFINE(push_notification, "Push Notification Plugin",
>>>> VERSION,
>>>>
>>>
>>> Regards,
>>> -Denis
>> I hope this clarifies the reason for this patch. I deeply apologize for
>> not including this information with the patch submission.
>>
>> Best regards // John Ernberg
>>
>
> Regards,
> -Denis
I think we can drop this patch set as it's most likely a fault on my 
part that causes the problem. If this happens to be an actual problem 
upstream under some circumstances, I will submit a new version of this set.

Best regards // John Ernberg

Re: [PATCH 1/5] modem: clean up atoms on modem_unregister

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

Hi John,

On 04/22/2016 08:07 AM, John Ernberg wrote:
> From: John Ernberg <john.ernberg@actia.se>
>
> This resolves a crash that can happen when a e.g. usb modem is removed.
> ---
>   src/modem.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/src/modem.c b/src/modem.c
> index a89fa48..4968839 100644
> --- a/src/modem.c
> +++ b/src/modem.c
> @@ -2071,6 +2071,9 @@ static void modem_unregister(struct ofono_modem *modem)
>   	if (modem->powered == TRUE)
>   		set_powered(modem, FALSE);
>

So in theory, set_powered calls flush_atoms...

> +	if (modem->atoms)
> +		flush_atoms(modem, MODEM_STATE_POWER_OFF);
> +

So why is this needed?

Do you have a stack trace handy?

>   	__ofono_watchlist_free(modem->atom_watches);
>   	modem->atom_watches = NULL;
>
>

Regards,
-Denis

Re: [PATCH 1/5] modem: clean up atoms on modem_unregister

[John Ernberg]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

Hi Denis,

On 04/22/2016 09:25 PM, Denis Kenzior wrote:
> Hi John,
>
> On 04/22/2016 08:07 AM, John Ernberg wrote:
>> From: John Ernberg <john.ernberg@actia.se>
>>
>> This resolves a crash that can happen when a e.g. usb modem is removed.
>> ---
>>   src/modem.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/src/modem.c b/src/modem.c
>> index a89fa48..4968839 100644
>> --- a/src/modem.c
>> +++ b/src/modem.c
>> @@ -2071,6 +2071,9 @@ static void modem_unregister(struct ofono_modem 
>> *modem)
>>       if (modem->powered == TRUE)
>>           set_powered(modem, FALSE);
>>
>
> So in theory, set_powered calls flush_atoms...
>
>> +    if (modem->atoms)
>> +        flush_atoms(modem, MODEM_STATE_POWER_OFF);
>> +
>
> So why is this needed?
>
> Do you have a stack trace handy?
>
>> __ofono_watchlist_free(modem->atom_watches);
>>       modem->atom_watches = NULL;
>>
>>
>
> Regards,
> -Denis
I cannot reproduce this one where the atoms remained anymore and I don't 
seem to have any saved stack traces.
So let's drop this part of the series and assume PEBKAC for now.

Best regards // John Ernberg