[asterisk-dev] [Code Review] 3891: Fix Lua regression caused by me fixing ASTERISK-23818 incorrectly.

George Joseph reviewboard at asterisk.org
Tue Aug 5 19:50:02 CDT 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3891/
-----------------------------------------------------------

Review request for Asterisk Developers and Matt Jordan.


Bugs: ASTERISK-23818
    https://issues.asterisk.org/jira/browse/ASTERISK-23818


Repository: Asterisk


Description
-------

ASTERISK-23818 (lua contexts being overwritten by contexts of the same name in pbx_config) surfaced because pbx_lua, having the AST_MODFLAG_GLOBAL_SYMBOLS set, was always force loaded before pbx_config.  Since I couldn't find any reason for pbx_lua to export it's symbols to the rest of Asterisk, I simply changed the flag to AST_MODFLAG_DEFAULT.  Problem solved.  What I didn't realize was that the symbols need to be exported not because Asterisk needs them but because any external Lua modules like luasql.mysql need the base Lua language APIs exported (ASTERISK-17279).

Back to ASTERISK-23818...  It looks like there's an issue in pbx.c where context_merge was only merging includes, switches and ignore patterns if the context was already existing AND has extensions, or if the context was brand new.  If pbx_lua is loaded before pbx_config, the context will exist BUT pbx_lua, being implemented as a switch, will never place extensions in it, just the switch statement.  The result is that when pbx_config loads, it never merges the switch statement created by pbx_lua into the final context.

This patch sets pbx_lua's modflag back to AST_MODFLAG_GLOBAL_SYMBOLS and adds an "else if" in context_merge that catches the case where an existing context has includes, switchs or ingore patterns but no actual extensions.

For later...  "dialplan show" always shows extensions, includes, then switches but pbx_find_extension actually processes extensions, switches then includes.  Which is correct?

Also, it appears that switches and includes are always inserted at the tail of their respective lists, but the lists are searched in reverse.  The result is that the last module loaded is searched for a matching extension first.  Is this expected behavior?

This patch needs to be applied from 1.8 through trunk.


Diffs
-----

  branches/1.8/pbx/pbx_lua.c 420123 
  branches/1.8/main/pbx.c 420123 

Diff: https://reviewboard.asterisk.org/r/3891/diff/


Testing
-------

Ran the TestSuite before and after with the same results.  461 run, 397 passed, 64 failed.

Confirmed that external Lua modules properly initialize.  I used luasql.mysql as a test.

Ran tests with various merge scenarios between pbx_config, pbx_lua and pbx_ael to make sure that where contexts overlap, includes, switches, and ingore patterns are preserved in the merge process.  Extensions are different...  pbx_config and pbx_ael share the same global dialplan so the first definition of context/exten/priority wins and subsequent are rejected.  pbx_lua maintains it's own environment so even if it happens to define the same context/exten as pbx_config or pbx_ael it'll never be called because pbx_find_extension will never search pbx_lua unless the global table is exhausted.


Thanks,

George Joseph

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140806/a30f8c6c/attachment.html>


More information about the asterisk-dev mailing list