[Asterisk-code-review] loader: Use MODULEINFO tag to determine dlopen order (asterisk[master])
George Joseph
asteriskteam at digium.com
Mon Jan 25 19:18:40 CST 2016
George Joseph has uploaded a new change for review.
https://gerrit.asterisk.org/2095
Change subject: loader: Use MODULEINFO tag to determine dlopen order
......................................................................
loader: Use MODULEINFO tag to determine dlopen order
The current process doesn't work very well when one module marked
AST_MODFLAG_GLOBAL_SYMBOLS depends on another module also marked
AST_MODFLAG_GLOBAL_SYMBOLS. In this case the order dlopen is called is the
order the OS returns them from an opendir call on the modules directory
which is non-deterministic. Although we use the RTLD_LAZY flag with dlopen,
that only defers resolution for function calls. If moduleA references
a POINTER to a function in moduleB, then moduleB MUST be loaded before
moduleA. This does happen with some pjproject calls.
Since we have the correct dependency tree in menuselect-tree at compile
time, this patch filters the menuselect-tree xml file through xsltproc
to produce a Makefile that represents the dependencies, then calls
'make all' on it to produce a header file containing an array of module
names in the proper load order.
loader.c includes that header and at runtime adds the contents to the
load_order list after modules.conf has been processed but before the
modules are read from the modules directory. As the modules directory
is read, pre-existing logic ignores any that are already in the list
so only new modules like third-party modules are picked up. Pre-existing
logic also removes any modules ones marked 'noload' in modules.conf.
The final list now only requires 1 pass instead of 1 for global symbols
and one for the rest. This in turn prevents the confusing messages we
sometimes see when a module fails to load the first time but silently
succeedes the second time.
NOTE: This patch DOES NOT alter the AST_MODFLAG_LOAD_ORDER/load_pri
processing. Modules still have their 'load' functions called in the
order specified by load_pri.
xsltproc is used to process menuselect-tree. It was already included
in install_prereq for RH and BSD based systems. Since I couldn't
verify it was incldued for Debian based systems, I added the xsltproc
package explicitly.
res_pjsip_pubsub and res_pjsip_session had incorrect load_pri settings
that placed them after modules that needed them to be initialized so
they were tweaked.
Change-Id: If7c84242a5fe2a433b7886505641c5d661111823
---
M Makefile
A build_tools/make_loaderdeps.xslt
M configure
M configure.ac
M contrib/scripts/install_prereq
M include/asterisk/.gitignore
M main/loader.c
M res/res_pjsip_pubsub.c
M res/res_pjsip_session.c
9 files changed, 109 insertions(+), 10 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/95/2095/1
diff --git a/Makefile b/Makefile
index b1de3d2..8f0330a 100644
--- a/Makefile
+++ b/Makefile
@@ -364,7 +364,7 @@
+@$(SUBMAKE) $(MOD_SUBDIRS_EMBED_LDFLAGS)
+@$(SUBMAKE) $(MOD_SUBDIRS_EMBED_LIBS)
-$(SUBDIRS): makeopts .lastclean main/version.c include/asterisk/build.h include/asterisk/buildopts.h defaults.h makeopts.embed_rules
+$(SUBDIRS): makeopts .lastclean main/version.c include/asterisk/build.h include/asterisk/buildopts.h include/asterisk/module_loadorder.h defaults.h makeopts.embed_rules
ifeq ($(findstring $(OSARCH), mingw32 cygwin ),)
ifeq ($(shell grep ^MENUSELECT_EMBED=$$ menuselect.makeopts 2>/dev/null),)
@@ -446,6 +446,7 @@
rm -rf autom4te.cache
rm -f include/asterisk/autoconfig.h
rm -f include/asterisk/buildopts.h
+ rm -f include/asterisk/module_loadorder.h
rm -rf doc/api
rm -f doc/asterisk-ng-doxygen
rm -f build_tools/menuselect-deps
@@ -995,6 +996,9 @@
@cat sounds/sounds.xml >> $@
@echo "</menu>" >> $@
+include/asterisk/module_loadorder.h: menuselect-tree menuselect.makeopts
+ @xsltproc build_tools/make_loaderdeps.xslt menuselect-tree | make -s -f- all > include/asterisk/module_loadorder.h 2>/dev/null
+
# We don't want to require Python or Pystache for every build, so this is its
# own target.
ari-stubs:
diff --git a/build_tools/make_loaderdeps.xslt b/build_tools/make_loaderdeps.xslt
new file mode 100644
index 0000000..6855f00
--- /dev/null
+++ b/build_tools/make_loaderdeps.xslt
@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
+<xsl:output method="text" omit-xml-declaration="yes" indent="no"/>
+<xsl:template match="/">
+<xsl:variable name="cats" select="'ADDONS APPS BRIDGES CDR CEL CHANNELS CODECS FORMATS FUNCS PBX RES TESTS'"/>
+<xsl:text disable-output-escaping="yes">
+
+%:
+ @echo -e '\t"$@.so",'
+
+header:
+ @echo "/* include/asterisk/module_loadorder.h. Generated from make */"
+ @echo "/* This file must be included only by main/loader.c */"
+ @echo ""
+ @echo "#ifndef MODULE_LOADORDER_H"
+ @echo "#define MODULE_LOADORDER_H"
+ @echo ""
+ @echo "static const char *module_loadorder[] = {"
+
+footer:
+ @echo '};'
+ @echo "#endif"
+
+</xsl:text>
+
+<xsl:for-each select="menu/category[contains($cats, substring(@name, 12))]/member">
+<xsl:value-of select="@name"/>:<xsl:for-each select="depend[/menu/category[contains(@name, 'CFLAGS') != 1]/member/@name = .]"><xsl:text> </xsl:text><xsl:value-of select="."/></xsl:for-each>
+<xsl:for-each select="use[/menu/category[contains(@name, 'CFLAGS') != 1]/member/@name = .]"><xsl:text> </xsl:text><xsl:value-of select="."/></xsl:for-each>
+<xsl:text>
+</xsl:text>
+</xsl:for-each>
+include menuselect.makeopts
+
+ALLMODS = <xsl:for-each select="menu/category[contains($cats, substring(@name, 12))]/member"> <xsl:value-of select="@name"/><xsl:text> </xsl:text></xsl:for-each>
+
+NOTNEEDED = $(foreach cat,<xsl:value-of select="$cats"/>,$(MENUSELECT_$(cat)))
+
+all: header $(filter-out $(NOTNEEDED),$(ALLMODS)) footer
+ @:
+</xsl:template>
+</xsl:stylesheet>
\ No newline at end of file
diff --git a/configure b/configure
index fc76131..bb804d2 100755
--- a/configure
+++ b/configure
@@ -1176,6 +1176,7 @@
FETCH
ALEMBIC
GIT
+XSLTPROC
XMLSTARLET
XMLLINT
KPATHSEA
@@ -7439,6 +7440,50 @@
fi
+# Extract the first word of "xsltproc", so it can be a program name with args.
+set dummy xsltproc; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_XSLTPROC+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $XSLTPROC in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_XSLTPROC="$XSLTPROC" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_XSLTPROC="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ test -z "$ac_cv_path_XSLTPROC" && ac_cv_path_XSLTPROC=":"
+ ;;
+esac
+fi
+XSLTPROC=$ac_cv_path_XSLTPROC
+if test -n "$XSLTPROC"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $XSLTPROC" >&5
+$as_echo "$XSLTPROC" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+if test "x${XSLTPROC}" = "x:" ; then
+ as_fn_error $? "xsltproc was not found" "$LINENO" 5
+fi
# Extract the first word of "git", so it can be a program name with args.
set dummy git; ac_word=$2
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
diff --git a/configure.ac b/configure.ac
index e7b7e57..f832010 100644
--- a/configure.ac
+++ b/configure.ac
@@ -278,6 +278,10 @@
AC_PATH_PROG([KPATHSEA], [kpsewhich], :)
AC_PATH_PROG([XMLLINT], [xmllint], :)
AC_PATH_PROG([XMLSTARLET], [xmlstarlet], :)
+AC_PATH_PROG([XSLTPROC], [xsltproc], :)
+if test "x${XSLTPROC}" = "x:" ; then
+ AC_MSG_ERROR(xsltproc was not found)
+fi
AC_PATH_PROG([GIT], [git], :)
AC_PATH_PROG([ALEMBIC], [alembic], :)
if test "${WGET}" != ":" ; then
diff --git a/contrib/scripts/install_prereq b/contrib/scripts/install_prereq
index 06eb05e..2b0d75e 100755
--- a/contrib/scripts/install_prereq
+++ b/contrib/scripts/install_prereq
@@ -28,7 +28,7 @@
PACKAGES_DEBIAN="$PACKAGES_DEBIAN libopenh323-dev libvpb-dev libgtk2.0-dev libmysqlclient-dev libbluetooth-dev libradiusclient-ng-dev freetds-dev"
PACKAGES_DEBIAN="$PACKAGES_DEBIAN libsnmp-dev libiksemel-dev libcorosync-dev libnewt-dev libpopt-dev libical-dev libspandsp-dev libjack-dev"
PACKAGES_DEBIAN="$PACKAGES_DEBIAN libresample-dev libc-client-dev binutils-dev libsrtp-dev libgsm1-dev libedit-dev doxygen libjansson-dev libldap-dev"
-PACKAGES_DEBIAN="$PACKAGES_DEBIAN subversion git libxslt1-dev"
+PACKAGES_DEBIAN="$PACKAGES_DEBIAN subversion git libxslt1-dev xsltproc"
PACKAGES_RH="automake gcc gcc-c++ ncurses-devel openssl-devel libxml2-devel unixODBC-devel libcurl-devel libogg-devel libvorbis-devel speex-devel"
PACKAGES_RH="$PACKAGES_RH spandsp-devel freetds-devel net-snmp-devel iksemel-devel corosynclib-devel newt-devel popt-devel libtool-ltdl-devel lua-devel"
PACKAGES_RH="$PACKAGES_RH sqlite-devel libsqlite3x-devel radiusclient-ng-devel portaudio-devel postgresql-devel libresample-devel neon-devel libical-devel"
diff --git a/include/asterisk/.gitignore b/include/asterisk/.gitignore
index ae33b3c..ead6349 100644
--- a/include/asterisk/.gitignore
+++ b/include/asterisk/.gitignore
@@ -1,3 +1,4 @@
autoconfig.h
build.h
buildopts.h
+module_loadorder.h
diff --git a/main/loader.c b/main/loader.c
index 954b288..4f8c899 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -57,6 +57,7 @@
#include "asterisk/app.h"
#include "asterisk/test.h"
#include "asterisk/sounds_index.h"
+#include "asterisk/module_loadorder.h"
#include <dlfcn.h>
@@ -1244,6 +1245,7 @@
int res = 0;
struct ast_flags config_flags = { 0 };
int modulecount = 0;
+ int i;
#ifdef LOADABLE_MODULES
struct dirent *dirent;
@@ -1299,6 +1301,12 @@
}
#ifdef LOADABLE_MODULES
+
+ /* Add the modules from the build-time array */
+ for (i = 0; i < ARRAY_LEN(module_loadorder); i++) {
+ add_to_load_order(module_loadorder[i], &load_order, 0);
+ }
+
/* if we are allowed to load dynamic modules, scan the directory for
for all available modules and add them as well */
if ((dir = opendir(ast_config_AST_MODULE_DIR))) {
@@ -1357,12 +1365,7 @@
if (load_count)
ast_log(LOG_NOTICE, "%u modules will be loaded.\n", load_count);
- /* first, load only modules that provide global symbols */
- if ((res = load_resource_list(&load_order, 1, &modulecount)) < 0) {
- goto done;
- }
-
- /* now load everything else */
+ /* now load everything */
if ((res = load_resource_list(&load_order, 0, &modulecount)) < 0) {
goto done;
}
diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index a703e29..0e48009 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -4508,5 +4508,5 @@
.support_level = AST_MODULE_SUPPORT_CORE,
.load = load_module,
.unload = unload_module,
- .load_pri = AST_MODPRI_CHANNEL_DEPEND,
+ .load_pri = AST_MODPRI_CHANNEL_DEPEND - 2,
);
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 7f13467..0e86470 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -2904,5 +2904,6 @@
.support_level = AST_MODULE_SUPPORT_CORE,
.load = load_module,
.unload = unload_module,
- .load_pri = AST_MODPRI_APP_DEPEND,
+ .load_pri = AST_MODPRI_CHANNEL_DEPEND,
);
+
--
To view, visit https://gerrit.asterisk.org/2095
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If7c84242a5fe2a433b7886505641c5d661111823
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
More information about the asterisk-code-review
mailing list