Summary:ASTERISK-11052: [patch] chan_mobile.c has no mutexes for data accessed by different threads
Reporter:non-poster (non-poster)Labels:
Date Opened:2007-12-15 23:21:14.000-0600Date Closed:2008-07-21 17:06:45
Versions:Frequency of
Environment:Attachments:( 0) chan_mobile-rwlocks.diff
Description:There are no mutexes in chan_mobile.c!  Multiple threads access the same data, eg the data in the mbl_pvt struct in the devices list.  (To compare, almost all of the "standard" channel modules use mutexes.)


Comments:By: Dave Bowerman (dbowerman) 2007-12-23 01:30:24.000-0600

this is by design, as i dont believe its an issue in this case.

feel free to re-open if you think this is causing chan_mobile to fail.

By: non-poster (non-poster) 2007-12-26 23:36:16.000-0600

Wow, seriously?  Why would multi-threaded software be designed to allow threads to clobber variables accessed simultaneously by other threads?  This is _bad_ design.

By: Dave Bowerman (dbowerman) 2008-01-08 06:53:31.000-0600

can you provide an example of where this is happening and causing chan_mobile to fail?

By: Vladimir Latyshev (latysheff) 2008-01-11 05:26:41.000-0600

Perhaps, this makes impossible recieving sms during voice call. Even if I patch code so that I restore channel state after reading SMS, call is hang up because on finished reading SMS you create channel with the same pvt struct.

By: Jason Parker (jparker) 2008-01-15 14:34:53.000-0600

I just uploaded a patch that I would like to have tested by somebody.  While this isn't specifically what you were talking about, I think it may be useful.

It adds rwlocking to the devices and adapters lists.

By: Josef Liska (phokz) 2008-02-04 07:43:25.000-0600

I tested this with latest trunk of asterisk (r102238) and latest chan_mobile (r507), and at least disconnect on incoming sms during the call is still there.

I do not know about any other scenario I should test. If you have some ideas what to test I'll do it.

By: jmls (jmls) 2008-02-17 13:02:11.000-0600

qwell, any further thoughts ?

By: Digium Subversion (svnbot) 2008-05-09 16:40:16

Repository: asterisk-addons
Revision: 597

U   trunk/channels/chan_mobile.c

r597 | qwell | 2008-05-09 16:40:15 -0500 (Fri, 09 May 2008) | 8 lines

Add locking (also switch to rw, because they are awesome) to adapters and devices lists.

(related to issue ASTERISK-11052)
Reported by: non-poster
     chan_mobile-rwlocks.diff uploaded by qwell (license 4)
Tested by: qwell



By: Tilghman Lesher (tilghman) 2008-07-16 10:54:48

dbowerman: any further thoughts?

By: Tilghman Lesher (tilghman) 2008-07-21 17:06:44

As there has been no further comment, I'm closing this as fixed by Qwell's patch.