Re: [MM] [PATCH] bearer: deny disconnect request if there is a cancel outstanding

[Aleksander Morgado <aleksander lanedo com>]

On 07/30/2012 10:10 PM, Thieu Le wrote:
> Hi Aleksander,
> 
> I've tried your patch below and mm-next did not hang during my test
> runs.  Although it didn't happen during my test passes, I am wondering
> if the bearer DISCONNECTING status can be overwritten if the modem
> changes state after the call to mm_bearer_disconnect()?
> 
> If there is no possibility of overwriting the DISCONNECTING status,
> then we can go with your approach.
> 

When the connection sequence gets cancelled the state should go like this:
  CONNECTING->DISCONNECTING->DISCONNECTED

The final 'CONNECTED' state is only reached when the connection sequence
finished *and* there was no cancellation. If the connection sequence
gets finished but we were request to cancel, the state should still skip
the 'CONNECTED' state fully.

Pushed a patch with the early DISCONNECTING state.

Cheers!


> Thanks,
> Thieu
> 
> 
> On Mon, Jul 30, 2012 at 5:56 AM, Aleksander Morgado
> <aleksander lanedo com> wrote:
>> Hey Ben & Thieu,
>>
>>> ---
>>>  src/mm-bearer.c |    3 ++-
>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/mm-bearer.c b/src/mm-bearer.c
>>> index 3cce948..3b1be8c 100644
>>> --- a/src/mm-bearer.c
>>> +++ b/src/mm-bearer.c
>>> @@ -450,7 +450,8 @@ mm_bearer_disconnect (MMBearer *self,
>>>      }
>>>
>>>      /* If already disconnecting, return error, don't allow a second request. */
>>> -    if (self->priv->status == MM_BEARER_STATUS_DISCONNECTING) {
>>> +    if (self->priv->status == MM_BEARER_STATUS_DISCONNECTING ||
>>> +        self->priv->disconnect_signal_handler) {
>>>          g_simple_async_result_set_error (
>>>              simple,
>>>              MM_CORE_ERROR,
>>>
>>
>> Just wondering if it's not better to change the bearer state to
>> 'disconnecting' when we cancel the connection attempt, something like:
>>
>> diff --git a/src/mm-bearer.c b/src/mm-bearer.c
>> index 3cce948..d8e681f 100644
>> --- a/src/mm-bearer.c
>> +++ b/src/mm-bearer.c
>> @@ -466,6 +466,8 @@ mm_bearer_disconnect (MMBearer *self,
>>      /* If currently connecting, try to cancel that operation, and wait
>> to get
>>       * disconnected. */
>>      if (self->priv->status == MM_BEARER_STATUS_CONNECTING) {
>> +        bearer_update_status (self, MM_BEARER_STATUS_DISCONNECTING);
>> +
>>          /* We MUST ensure that we get to DISCONNECTED */
>>          g_cancellable_cancel (self->priv->connect_cancellable);
>>
>>
>> I believe that it should work; could you guys test that?
>>
>> Cheers!
>>
>> --
>> Aleksander


-- 
Aleksander