| Summary: | ASTERISK-13948: Thread-specific vm_state tracking issue if a voicemail is left immediately after a restart. | ||
| Reporter: | James Rothenberger (jaroth) | Labels: | |
| Date Opened: | 2009-04-13 15:16:40 | Date Closed: | 2009-05-19 15:19:17 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Applications/app_voicemail/IMAP |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) app_voicemail.c.patch | |
| Description: | When calling vmexec to leave a voicemail, pthread_getspecific() is called before pthread_setspecific() is called, thereby returning an undefined vm_state structure. The following functions (get_vm_state_by_imapuser, get_vm_state_by_mailbox, create_vm_state_from_user) could call pthread_getspecific during vmexec. The specific behavior is that if you leave a message for a mailbox that has not been accessed since a restart, pthread_getspecific will not return a valid vm_state and therefor message counts will be incorrect (all zeros). | ||
| Comments: | By: James Rothenberger (jaroth) 2009-04-13 15:19:30 Only externnotify uses the current mailbox count when leaving a message, so this is only place that the problem manifests itself. By: Mark Sirota (msirota) 2009-04-13 16:25:39 More detail about this bug (sorry about the verbosity, I hope it's helpful): Again, the case in question is when a new message is left for an IMAP mailbox that has not been read since the last restart, and the goal is to deliver accurate message counts to externnotify. run_externnotify() is called by notify_new_message() from leave_voicemail(). In the case where a message is left for a mailbox that has not yet been read since Asterisk restarted, leave_voicemail() creates a new vm_state using create_vm_state_from_user(). This vm_state currently has no message counts (all zeros). We would like to set the message counts in this vm_state using messagecount(). messagecount() first searches for an interactive vm_state. There shouldn't be one for this mailbox, because the first thing that happened was to leave a message, so no interactive vm_state should exist. However, messagecount() finds one anyway. It would appear that it finds this phantom vm_state by calling get_vm_state_by_imapuser() or get_vm_state_by_mailbox() with the Interactive flag set to True. Because Interactive is true, these functions call pthread_getspecific(), which always returns a vm_state. However, pthread_setspecific() is only called for interactive sessions. There hasn't been an interactive session for this mailbox yet, so pthread_getspecific() is returning a phantom vm_state. Two possible approaches to fix this: (1) call pthread_setspecific() even for non-interactive sessions (2) avoid calling pthread_getspecific() for non-interactive sessions. I can't recommend a specific solution because I don't understand exactly problem this pthread_setspecific()/pthread_getspecific() is there to solve. Either way, our goal is to have messagecount() find the non-interactive vm_state created by leave_voicemail(), rather than finding a phantom interactive vm_state that doesn't really exist. Thanks. By: Tilghman Lesher (tilghman) 2009-04-15 18:25:51 Linking to development list discussion here: http://lists.digium.com/pipermail/asterisk-dev/2009-April/037831.html By: Mark Sirota (msirota) 2009-04-16 12:32:44 Per the discussion linked above, the problem is that the pthread key is not initialized before calling pthread_getspecific() in this case. Following Russell's suggestion, the solution is to call pthread_once() before pthread_getspecific() in each case where it is possible that it might be called before pthread_setspecific(). 3-line patch attached. Thanks, Russell. By: Jeff Phelps (blargman) 2009-05-07 17:29:57 This patch seems to have worked for me... I no longer get incorrect VM counts after a restart when VM is left before a user has logged into their voicemail to check it (which I might add my users NEVER do)... By: Digium Subversion (svnbot) 2009-05-19 15:12:21 Repository: asterisk Revision: 195520 U branches/1.4/apps/app_voicemail.c ------------------------------------------------------------------------ r195520 | tilghman | 2009-05-19 15:12:20 -0500 (Tue, 19 May 2009) | 7 lines Ensure thread keys are initialized before attempting to access them. (closes issue ASTERISK-13948) Reported by: jaroth Patches: app_voicemail.c.patch uploaded by msirota (license 758) Tested by: msirota, BlargMaN ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=195520 By: Digium Subversion (svnbot) 2009-05-19 15:16:02 Repository: asterisk Revision: 195521 _U trunk/ U trunk/apps/app_voicemail.c ------------------------------------------------------------------------ r195521 | tilghman | 2009-05-19 15:16:02 -0500 (Tue, 19 May 2009) | 14 lines Merged revisions 195520 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r195520 | tilghman | 2009-05-19 15:12:20 -0500 (Tue, 19 May 2009) | 7 lines Ensure thread keys are initialized before attempting to access them. (closes issue ASTERISK-13948) Reported by: jaroth Patches: app_voicemail.c.patch uploaded by msirota (license 758) Tested by: msirota, BlargMaN ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=195521 By: Digium Subversion (svnbot) 2009-05-19 15:17:11 Repository: asterisk Revision: 195522 _U branches/1.6.0/ U branches/1.6.0/apps/app_voicemail.c ------------------------------------------------------------------------ r195522 | tilghman | 2009-05-19 15:17:11 -0500 (Tue, 19 May 2009) | 21 lines Merged revisions 195521 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r195521 | tilghman | 2009-05-19 15:16:01 -0500 (Tue, 19 May 2009) | 14 lines Merged revisions 195520 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r195520 | tilghman | 2009-05-19 15:12:20 -0500 (Tue, 19 May 2009) | 7 lines Ensure thread keys are initialized before attempting to access them. (closes issue ASTERISK-13948) Reported by: jaroth Patches: app_voicemail.c.patch uploaded by msirota (license 758) Tested by: msirota, BlargMaN ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=195522 By: Digium Subversion (svnbot) 2009-05-19 15:18:12 Repository: asterisk Revision: 195526 _U branches/1.6.1/ U branches/1.6.1/apps/app_voicemail.c ------------------------------------------------------------------------ r195526 | tilghman | 2009-05-19 15:18:12 -0500 (Tue, 19 May 2009) | 21 lines Merged revisions 195521 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r195521 | tilghman | 2009-05-19 15:16:01 -0500 (Tue, 19 May 2009) | 14 lines Merged revisions 195520 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r195520 | tilghman | 2009-05-19 15:12:20 -0500 (Tue, 19 May 2009) | 7 lines Ensure thread keys are initialized before attempting to access them. (closes issue ASTERISK-13948) Reported by: jaroth Patches: app_voicemail.c.patch uploaded by msirota (license 758) Tested by: msirota, BlargMaN ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=195526 By: Digium Subversion (svnbot) 2009-05-19 15:19:16 Repository: asterisk Revision: 195531 _U branches/1.6.2/ U branches/1.6.2/apps/app_voicemail.c ------------------------------------------------------------------------ r195531 | tilghman | 2009-05-19 15:19:15 -0500 (Tue, 19 May 2009) | 21 lines Merged revisions 195521 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r195521 | tilghman | 2009-05-19 15:16:01 -0500 (Tue, 19 May 2009) | 14 lines Merged revisions 195520 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r195520 | tilghman | 2009-05-19 15:12:20 -0500 (Tue, 19 May 2009) | 7 lines Ensure thread keys are initialized before attempting to access them. (closes issue ASTERISK-13948) Reported by: jaroth Patches: app_voicemail.c.patch uploaded by msirota (license 758) Tested by: msirota, BlargMaN ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=195531 | ||