[Telephony] Should initialize call->timer for inbound call

RESOLVED FIXED in 2.2 S2 (19dec)

Status

Firefox OS
Emulator
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: aknow, Assigned: aknow)

Tracking

unspecified
2.2 S2 (19dec)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
I spent whole day debugging this problem ...

It might causes emulator crash in some cases.

Think about:

1. Have 1 incoming call
2. Have 1 outgoing call.

Then, in emulator
- modem->calls[0]: incoming call
- modem->calls[1]: outgoing call, call->timer is set

3. disconnect incoming call.

In amodem_free_call, we perform the memmove() that lead to the following changes
- modem->calls[0]: outgoing call, call->timer is set
- modem->calls[1]: outgoing call, call->timer is set
- modem->call_count = 1

4. have another incoming call
- modem->calls[0]: outgoing call, call->timer is set
- modem->calls[1]: incoming call, call->timer is set because it contains the previous value!!

Note that modem->calls[0]->timer == modem->calls[1]->timer

5. disconnect calls[1] and calls[0]

sys_timer_destroy() will be called for same timer address twice!!

Timer resource is implemented by a linked list of free timer. When we free the timer, it is prepend to the list. 

timer->next    = _s_free_timers;

So, if timer and _s_free_timers is point to the same position, it introduces a self loop.

Next time, when we allocate the timer.

sys_timer_alloc( void )              
{                                    
    SysTimer  timer = _s_free_timers;
                                     
    if (timer != NULL) {             
        _s_free_timers = timer->next;
        timer->next    = NULL;       
        timer->timer   = NULL;       
    }                                
    return timer;                    
}                                                                  

timer->next will set the remaining list to empty.

In the end, we lack enough timer resource next time and it cause the crash.
(Assignee)

Comment 1

4 years ago
Created attachment 8530206 [details] [review]
[platform_external_qemu] PR 121
Attachment #8530206 - Flags: review?(echen)
(Assignee)

Updated

4 years ago
Blocks: 1093014

Comment 2

4 years ago
Comment on attachment 8530206 [details] [review]
[platform_external_qemu] PR 121

Thank you for spending time on debugging this. :)
Attachment #8530206 - Flags: review?(echen) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://github.com/mozilla-b2g/platform_external_qemu/commit/6fa7a4936414ceb4055fd27f7a30e76790f834fb
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S2 (19dec)

Updated

4 years ago
Blocks: 1090359
(Assignee)

Comment 5

4 years ago
Created attachment 8539901 [details] [review]
(kk) [platform_external_qemu] PR 125
Attachment #8539901 - Flags: review+
(Assignee)

Comment 6

4 years ago
check-in needed for kk branch
=> (kk) [platform_external_qemu] PR 125
Keywords: checkin-needed

Updated

3 years ago
Blocks: 1124557

Updated

3 years ago
No longer blocks: 1124557
You need to log in before you can comment on or make changes to this bug.