| Summary: | ASTERISK-12855: [patch] Shared IMAP mailboxes can cause the server to crash | ||
| Reporter: | Howard Wilkinson (howardwilkinson) | Labels: | |
| Date Opened: | 2008-10-09 09:02:01 | Date Closed: | 2008-12-18 15:57:12.000-0600 |
| Priority: | Critical | Regression? | No |
| Status: | Closed/Complete | Components: | Applications/app_voicemail/IMAP |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) asterisk-1.4.21.2-appvoicemail-sharedimap-lock.patch ( 1) asterisk-1.4.21-appvoicemail-sharedimap.patch ( 2) BlargMaN_-_gdb.txt ( 3) BlargMaN_-_gdb2.txt | |
| Description: | If the IMAP mailbox used to hold the voice messages is shared amongst a number of users then the server can crash with problems inside the UW-IMAP C-Client code. This is caused by multiple threads trying to use the shared structure that the c-client code produces for the mailbox stream. The particular installation is using Cyrus Imap server, with authuser/authpassword set and multiple users having the same imapuser setting. ****** ADDITIONAL INFORMATION ****** This is fixed (very conservatively) by taking the Mutex lock every time a call into the UW library is performed. This is mainly mail_ calls but includes at least 1 imap_ call. | ||
| Comments: | By: Howard Wilkinson (howardwilkinson) 2008-10-09 09:05:42 I have attached a patch that fixes this problem and also adds a feature allowing the shared mailboxes to be used to share the messages as well. This feature adds a new option to the voicemail set. This option 'imapmbxid' can be set to a common key that should be used to identify the messages when saved and retrieved for this particular user. This allows a message left for one user to be retrieved by another user as though they are the same. I use this to provide a shared set of phones with identical behaviour but still to have separate passwords and greeting messages. By: Mark Michelson (mmichelson) 2008-10-09 18:30:29 howardwilkinson: Thanks for the patch. However, the imapmbxid has nothing to do with the bug reported. If you would like to submit the imapmbxid patch, please do so in a new bug report, marked as feature. This helps keep issues focused on what was originally reported. Thanks! As it is, the rest of the patch is really useful for fixing this issue. Thanks very much! By: Howard Wilkinson (howardwilkinson) 2008-10-11 06:19:16 I have uploaded a new patch which only contains the lock code and not my extension which exaggerated the problem in the first place. I will update the extension as a second patch with a feature request. This patch does not fix the under lying issue it just stops * from crashing when the code tries to enter the UW library twice. The underlying problem is that independent threads of the * code can try to update the same mailbox and trip over each other. This requires a larger locking regime and some additional logic to overcome as a failure but raises a problem with this approach which arises if 2 users try to access the mailbox at the same time. In the circumstance, they will get told that there are 'x' new messages and may listen to the same one and both try to delete it. Or possibly one deletes before the other can access. The code as it stands means that the list of available messages is updated in 2 threads and less than predictable results could occur. With more intelligence in the call-backs from the UW library most of this could be removed as an issue. Careful coding of the access to the data retrieved from the IMAP server and a widening of the locks should remove any other logical inconsistencies. This would just leave the transaction issue where 2 users access at the same time but get told "exaggerated" numbers for available messages. It might be worth looking at adding an interface options that allows the system to issue a 'there are 1 or more messages available' prompt followed by a 'last message deleted by another user' if this circumstance arises. I will look at fixing the logical issue today but may only provide a half fix. By: Mark Michelson (mmichelson) 2008-10-20 14:42:10 I have referred the reporter of issue ASTERISK-12873 to the second patch on this issue since I think that at the root of it all, you're experiencing the same problems. I'm also adding a relationship between the two issues. By: Jeff Peeler (jpeeler) 2008-12-02 17:37:38.000-0600 I've reproduced this problem and added 13214 as being related. The patch here did seem to prevent any crashing. It would be sad to add so much locking though, but may be necessary: http://mailman2.u.washington.edu/pipermail/imap-uw/2008-March/001925.html I noticed on at least one occasion a thread got stuck in select called from the c-client code. About a minute later Asterisk crashed. I'm going to try the latest c-client code now since my first attempt was using what shipped with my distro. By: Jeff Peeler (jpeeler) 2008-12-02 18:37:44.000-0600 The latest c-client code imap-2007d also has this problem. By: Jeff Phelps (blargman) 2008-12-11 12:46:59.000-0600 Just wanted to let everyone know that I am using 1.6.1-beta3 with imap-2002e on Debian, and I am getting the issue as well. I applied the patch listed above by hand, as it would not apply correctly, and the crashes arevery much less frequent, but I still get crashes. I will try to upgrade to the imap-2007d and see if the issue persists. By: Jeff Peeler (jpeeler) 2008-12-12 18:22:04.000-0600 BlargMan: if you run asterisk in gdb and trace the code back up into Asterisk, is the c-client call protected by a lock? It's possible that some calls were missed. I'm beginning to think this might be the best we can do here. By: Jeff Phelps (blargman) 2008-12-15 11:37:25.000-0600 I added a Backtrace from gdb, but I'm not sure if I was doing it correctly. You'll notice at the end of the attaching process that it fails, and it then causes * to crash. Hope this is helpful, or maybe someone can help me get a better backtrace. For some reason I can't get * to give me a core file. By: Jeff Phelps (blargman) 2008-12-15 16:32:04.000-0600 I added a second Backtrace that should hopefully be much more helpful. By: Digium Subversion (svnbot) 2008-12-18 15:14:43.000-0600 Repository: asterisk Revision: 165767 U branches/1.4/apps/app_voicemail.c ------------------------------------------------------------------------ r165767 | tilghman | 2008-12-18 15:14:42 -0600 (Thu, 18 Dec 2008) | 8 lines Add mutexes around accesses to the IMAP library interface. This prevents certain crashes, especially when shared mailboxes are used. (closes issue ASTERISK-12855) Reported by: howardwilkinson Patches: asterisk-1.4.21.2-appvoicemail-sharedimap-lock.patch uploaded by howardwilkinson (license 590) Tested by: jpeeler ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=165767 By: Digium Subversion (svnbot) 2008-12-18 15:40:58.000-0600 Repository: asterisk Revision: 165797 _U trunk/ U trunk/apps/app_voicemail.c ------------------------------------------------------------------------ r165797 | tilghman | 2008-12-18 15:40:57 -0600 (Thu, 18 Dec 2008) | 15 lines Merged revisions 165767 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r165767 | tilghman | 2008-12-18 15:14:47 -0600 (Thu, 18 Dec 2008) | 8 lines Add mutexes around accesses to the IMAP library interface. This prevents certain crashes, especially when shared mailboxes are used. (closes issue ASTERISK-12855) Reported by: howardwilkinson Patches: asterisk-1.4.21.2-appvoicemail-sharedimap-lock.patch uploaded by howardwilkinson (license 590) Tested by: jpeeler ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=165797 By: Digium Subversion (svnbot) 2008-12-18 15:56:22.000-0600 Repository: asterisk Revision: 165806 _U branches/1.6.0/ U branches/1.6.0/apps/app_voicemail.c ------------------------------------------------------------------------ r165806 | tilghman | 2008-12-18 15:56:22 -0600 (Thu, 18 Dec 2008) | 22 lines Merged revisions 165797 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r165797 | tilghman | 2008-12-18 15:41:02 -0600 (Thu, 18 Dec 2008) | 15 lines Merged revisions 165767 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r165767 | tilghman | 2008-12-18 15:14:47 -0600 (Thu, 18 Dec 2008) | 8 lines Add mutexes around accesses to the IMAP library interface. This prevents certain crashes, especially when shared mailboxes are used. (closes issue ASTERISK-12855) Reported by: howardwilkinson Patches: asterisk-1.4.21.2-appvoicemail-sharedimap-lock.patch uploaded by howardwilkinson (license 590) Tested by: jpeeler ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=165806 By: Digium Subversion (svnbot) 2008-12-18 15:57:10.000-0600 Repository: asterisk Revision: 165808 _U branches/1.6.1/ U branches/1.6.1/apps/app_voicemail.c ------------------------------------------------------------------------ r165808 | tilghman | 2008-12-18 15:57:10 -0600 (Thu, 18 Dec 2008) | 22 lines Merged revisions 165797 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r165797 | tilghman | 2008-12-18 15:41:02 -0600 (Thu, 18 Dec 2008) | 15 lines Merged revisions 165767 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r165767 | tilghman | 2008-12-18 15:14:47 -0600 (Thu, 18 Dec 2008) | 8 lines Add mutexes around accesses to the IMAP library interface. This prevents certain crashes, especially when shared mailboxes are used. (closes issue ASTERISK-12855) Reported by: howardwilkinson Patches: asterisk-1.4.21.2-appvoicemail-sharedimap-lock.patch uploaded by howardwilkinson (license 590) Tested by: jpeeler ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=165808 | ||