Re: [PATCH] fix miss dbus reply issue

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Caiwen,

On 05/10/2011 05:31 AM, Caiwen Zhang wrote:
> When set modem offline or power off, if there is some other operation in progress,
> the dbus reply may lost. Always check the pending dbus message to make sure don't
> miss any reply.
> 
> This should be a common issue, this patch only fix the GPRS relatived.
> 
> ---
>  src/gprs.c |   17 ++++++++++++++++++-
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/src/gprs.c b/src/gprs.c
> index e221741..535432e 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -1363,6 +1363,10 @@ static gboolean context_dbus_unregister(struct pri_context *ctx)
>  	DBusConnection *conn = ofono_dbus_get_connection();
>  	char path[256];
>  
> +	if (ctx->pending != NULL)
> +		__ofono_dbus_pending_reply(&ctx->pending,
> +					__ofono_error_failed(ctx->pending));
> +

I don't really see the need for this change just yet, especially if the
gprs atom is moved to post_sim state.  Feel free to convince me otherwise ;)

>  	strcpy(path, ctx->path);
>  	idmap_put(ctx->gprs->pid_map, ctx->id);
>  
> @@ -2133,6 +2137,8 @@ void ofono_gprs_set_cid_range(struct ofono_gprs *gprs,
>  static void gprs_context_unregister(struct ofono_atom *atom)
>  {
>  	struct ofono_gprs_context *gc = __ofono_atom_get_data(atom);
> +	GSList *l;
> +	struct pri_context *ctx;
>  
>  	if (gc->gprs == NULL)
>  		return;
> @@ -2143,6 +2149,13 @@ static void gprs_context_unregister(struct ofono_atom *atom)
>  		gc->settings = NULL;
>  	}
>  
> +	for (l = gc->gprs->contexts; l; l = l->next) {
> +		ctx = l->data;
> +		if (ctx->context_driver == gc && ctx->pending != NULL)
> +			__ofono_dbus_pending_reply(&ctx->pending,
> +					__ofono_error_failed(ctx->pending));
> +	}
> +
>  	gc->gprs->context_drivers = g_slist_remove(gc->gprs->context_drivers,
>  							gc);
>  	gc->gprs = NULL;

I've fixed this part slightly differently in commit
4714d8199aacf0ba6be777a70e01b3016ba2aa5f.  Please have a look and let me
know if it solves your problems.

> @@ -2481,6 +2494,10 @@ static void gprs_unregister(struct ofono_atom *atom)
>  		gprs->netreg = NULL;
>  	}
>  
> +	if (gprs->pending != NULL)
> +		__ofono_dbus_pending_reply(&gprs->pending,
> +					__ofono_error_failed(gprs->pending));
> +
>  	ofono_modem_remove_interface(modem,
>  					OFONO_CONNECTION_MANAGER_INTERFACE);
>  	g_dbus_unregister_interface(conn, path,

Again, replying to pending messages on atom removal is not something I
want to do yet.  The application can easily cancel pending calls when a
particular interface goes away..

Regards,
-Denis