[asterisk-commits] core: Use eventfd for alert pipes on Linux when possible (asterisk[13])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue Apr 25 09:34:42 CDT 2017
George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/5497 )
Change subject: core: Use eventfd for alert pipes on Linux when possible
......................................................................
core: Use eventfd for alert pipes on Linux when possible
The primary win of switching to eventfd when possible is that it only
uses a single file descriptor while pipe() will use two. This means for
each bridge channel we're reducing the number of required file
descriptors by 1, and - if you're using timerfd - we also now have 1
less file descriptor per Asterisk channel.
The API is not ideal (passing int arrays), but this is the cleanest
approach I could come up with to maintain API/ABI.
I've also removed what I believe to be an erroneous code block that
checked the non-blocking flag on the pipe ends for each read. If the
file descriptor is 'losing' its non-blocking mode, it is because of a
bug somewhere else in our code.
In my testing I haven't seen any measurable difference in performance.
Change-Id: Iff0fb1573e7f7a187d5211ddc60aa8f3da3edb1d
---
M configure
M configure.ac
A include/asterisk/alertpipe.h
M include/asterisk/autoconfig.h.in
M include/asterisk/channel.h
A main/alertpipe.c
M main/bridge_channel.c
M main/channel_internal_api.c
M main/utils.c
9 files changed, 412 insertions(+), 203 deletions(-)
Approvals:
George Joseph: Looks good to me, but someone else must approve
Jenkins2: Verified
Joshua Colp: Looks good to me, approved
diff --git a/configure b/configure
index b9abc06..879ed6d 100755
--- a/configure
+++ b/configure
@@ -1326,6 +1326,7 @@
docdir
oldincludedir
includedir
+runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -1505,6 +1506,7 @@
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
+runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1757,6 +1759,15 @@
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
+ -runstatedir | --runstatedir | --runstatedi | --runstated \
+ | --runstate | --runstat | --runsta | --runst | --runs \
+ | --run | --ru | --r)
+ ac_prev=runstatedir ;;
+ -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
+ | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
+ | --run=* | --ru=* | --r=*)
+ runstatedir=$ac_optarg ;;
+
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1894,7 +1905,7 @@
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
- libdir localedir mandir
+ libdir localedir mandir runstatedir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
@@ -2047,6 +2058,7 @@
--sysconfdir=DIR read-only single-machine data [PREFIX/etc]
--sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
+ --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIR object code libraries [EPREFIX/lib]
--includedir=DIR C header files [PREFIX/include]
--oldincludedir=DIR C header files for non-gcc [/usr/include]
@@ -14655,7 +14667,7 @@
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -14701,7 +14713,7 @@
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -14725,7 +14737,7 @@
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -14770,7 +14782,7 @@
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -14794,7 +14806,7 @@
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -17782,6 +17794,41 @@
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if we have usable eventfd support" >&5
+$as_echo_n "checking if we have usable eventfd support... " >&6; }
+if test "$cross_compiling" = yes; then :
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "cannot run test program while cross compiling
+See \`config.log' for more details" "$LINENO" 5; }
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#include <sys/eventfd.h>
+int
+main ()
+{
+return eventfd(0, EFD_NONBLOCK | EFD_SEMAPHORE) == -1;
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_run "$LINENO"; then :
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+$as_echo "#define HAVE_EVENTFD 1" >>confdefs.h
+
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+fi
+rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
+ conftest.$ac_objext conftest.beam conftest.$ac_ext
+fi
+
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for compiler 'attribute pure' support" >&5
$as_echo_n "checking for compiler 'attribute pure' support... " >&6; }
diff --git a/configure.ac b/configure.ac
index 0458ce1..bf98ad3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1098,6 +1098,15 @@
AC_DEFINE([CONFIGURE_RAN_AS_ROOT], 1, [Some configure tests will unexpectedly fail if configure is run by a non-root user. These may be able to be tested at runtime.]))
fi
+AC_MSG_CHECKING([if we have usable eventfd support])
+AC_RUN_IFELSE(
+ [AC_LANG_PROGRAM([#include <sys/eventfd.h>],
+ [return eventfd(0, EFD_NONBLOCK | EFD_SEMAPHORE) == -1;])],
+ AC_MSG_RESULT(yes)
+ AC_DEFINE([HAVE_EVENTFD], 1, [Define to 1 if your system supports eventfd and the EFD_NONBLOCK and EFD_SEMAPHORE flags.]),
+ AC_MSG_RESULT(no)
+)
+
AST_GCC_ATTRIBUTE(pure)
AST_GCC_ATTRIBUTE(malloc)
AST_GCC_ATTRIBUTE(const)
diff --git a/include/asterisk/alertpipe.h b/include/asterisk/alertpipe.h
new file mode 100644
index 0000000..5ff854c
--- /dev/null
+++ b/include/asterisk/alertpipe.h
@@ -0,0 +1,159 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Sean Bright
+ *
+ * Sean Bright <sean.bright at gmail.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+#ifndef ASTERISK_ALERTPIPE_H
+#define ASTERISK_ALERTPIPE_H
+
+#include "asterisk/utils.h"
+
+typedef enum {
+ AST_ALERT_READ_SUCCESS = 0,
+ AST_ALERT_NOT_READABLE,
+ AST_ALERT_READ_FAIL,
+ AST_ALERT_READ_FATAL,
+} ast_alert_status_t;
+
+/*!
+ * \brief Initialize an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array to hold the alert pipe's file descriptors
+ *
+ * \return non-zero if a failure occurred, zero otherwise.
+ */
+int ast_alertpipe_init(int alert_pipe[2]);
+
+/*!
+ * \brief Close an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element containing the alert pipe's file descriptors
+ */
+void ast_alertpipe_close(int alert_pipe[2]);
+
+/*!
+ * \brief Read an event from an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \retval AST_ALERT_READ_SUCCESS on success
+ * \retval AST_ALERT_NOT_READABLE if the alert pipe is not readable
+ * \retval AST_ALERT_READ_FATAL if the alert pipe's file descriptors are in
+ * blocking mode, or a read error occurs.
+ */
+ast_alert_status_t ast_alertpipe_read(int alert_pipe[2]);
+
+/*!
+ * \brief Write an event to an alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return see write(2)
+ */
+ssize_t ast_alertpipe_write(int alert_pipe[2]);
+
+/*!
+ * \brief Consume all alerts written to the alert pipe
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \retval AST_ALERT_READ_SUCCESS on success
+ * \retval AST_ALERT_NOT_READABLE if the alert pipe is not readable
+ * \retval AST_ALERT_READ_FATAL if the alert pipe's file descriptors are in
+ * blocking mode, or a read error occurs.
+ */
+ast_alert_status_t ast_alertpipe_flush(int alert_pipe[2]);
+
+/*!
+ * \brief Sets the alert pipe file descriptors to default values
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ */
+AST_INLINE_API(
+void ast_alertpipe_clear(int alert_pipe[2]),
+{
+ alert_pipe[0] = alert_pipe[1] = -1;
+}
+)
+
+/*!
+ * \brief Determine if the alert pipe is readable
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return non-zero if the alert pipe is readable, zero otherwise.
+ */
+AST_INLINE_API(
+int attribute_pure ast_alertpipe_readable(int alert_pipe[2]),
+{
+ return alert_pipe[0] > -1;
+}
+)
+
+/*!
+ * \brief Determine if the alert pipe is writable
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return non-zero if the alert pipe is writable, zero otherwise.
+ */
+AST_INLINE_API(
+int attribute_pure ast_alertpipe_writable(int alert_pipe[2]),
+{
+ return alert_pipe[1] > -1;
+}
+)
+
+/*!
+ * \brief Get the alert pipe's read file descriptor
+ * \since 13.16.0
+ *
+ * \param p a two-element array containing the alert pipe's file descriptors
+ *
+ * \return -1 if the file descriptor is not initialized, a non-negative value
+ * otherwise.
+ */
+AST_INLINE_API(
+int attribute_pure ast_alertpipe_readfd(int alert_pipe[2]),
+{
+ return alert_pipe[0];
+}
+)
+
+/*!
+ * \brief Swap the file descriptors from two alert pipes
+ * \since 13.16.0
+ *
+ * \param p1 a two-element array containing an alert pipe's file descriptors
+ * \param p2 a two-element array containing an alert pipe's file descriptors
+ */
+AST_INLINE_API(
+void ast_alertpipe_swap(int alert_pipe_1[2], int alert_pipe_2[2]),
+{
+ SWAP(alert_pipe_1[0], alert_pipe_2[0]);
+ SWAP(alert_pipe_1[1], alert_pipe_2[1]);
+}
+)
+
+#endif /* ASTERISK_ALERTPIPE_H */
diff --git a/include/asterisk/autoconfig.h.in b/include/asterisk/autoconfig.h.in
index b675331..e7128e6 100644
--- a/include/asterisk/autoconfig.h.in
+++ b/include/asterisk/autoconfig.h.in
@@ -210,6 +210,10 @@
/* Define to 1 if you have the `euidaccess' function. */
#undef HAVE_EUIDACCESS
+/* Define to 1 if your system supports eventfd and the EFD_NONBLOCK and
+ EFD_SEMAPHORE flags. */
+#undef HAVE_EVENTFD
+
/* Define to 1 if you have the `exp' function. */
#undef HAVE_EXP
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 5765e75..b553bbe 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -123,6 +123,7 @@
#ifndef _ASTERISK_CHANNEL_H
#define _ASTERISK_CHANNEL_H
+#include "asterisk/alertpipe.h"
#include "asterisk/abstract_jb.h"
#include "asterisk/astobj2.h"
#include "asterisk/poll-compat.h"
@@ -4220,12 +4221,6 @@
void ast_channel_named_pickupgroups_set(struct ast_channel *chan, struct ast_namedgroups *value);
/* Alertpipe accessors--the "internal" functions for channel.c use only */
-typedef enum {
- AST_ALERT_READ_SUCCESS = 0,
- AST_ALERT_NOT_READABLE,
- AST_ALERT_READ_FAIL,
- AST_ALERT_READ_FATAL,
-} ast_alert_status_t;
int ast_channel_alert_write(struct ast_channel *chan);
int ast_channel_alert_writable(struct ast_channel *chan);
ast_alert_status_t ast_channel_internal_alert_flush(struct ast_channel *chan);
diff --git a/main/alertpipe.c b/main/alertpipe.c
new file mode 100644
index 0000000..fa6ec7b
--- /dev/null
+++ b/main/alertpipe.c
@@ -0,0 +1,166 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Sean Bright
+ *
+ * Sean Bright <sean.bright at gmail.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief Alert Pipe API
+ *
+ * \author Sean Bright
+ */
+
+#include "asterisk.h"
+
+#include <unistd.h>
+#include <fcntl.h>
+
+#ifdef HAVE_EVENTFD
+# include <sys/eventfd.h>
+#endif
+
+#include "asterisk/alertpipe.h"
+#include "asterisk/logger.h"
+
+int ast_alertpipe_init(int alert_pipe[2])
+{
+#ifdef HAVE_EVENTFD
+
+ int fd = eventfd(0, EFD_NONBLOCK | EFD_SEMAPHORE);
+ if (fd > -1) {
+ alert_pipe[0] = alert_pipe[1] = fd;
+ return 0;
+ }
+
+ ast_log(LOG_WARNING, "Failed to create alert pipe with eventfd(), falling back to pipe(): %s\n",
+ strerror(errno));
+ ast_alertpipe_clear(alert_pipe);
+
+#endif
+
+ if (pipe(alert_pipe)) {
+ ast_log(LOG_WARNING, "Failed to create alert pipe: %s\n", strerror(errno));
+ return -1;
+ } else {
+ int flags = fcntl(alert_pipe[0], F_GETFL);
+ if (fcntl(alert_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
+ ast_log(LOG_WARNING, "Failed to set non-blocking mode on alert pipe: %s\n",
+ strerror(errno));
+ ast_alertpipe_close(alert_pipe);
+ return -1;
+ }
+ flags = fcntl(alert_pipe[1], F_GETFL);
+ if (fcntl(alert_pipe[1], F_SETFL, flags | O_NONBLOCK) < 0) {
+ ast_log(LOG_WARNING, "Failed to set non-blocking mode on alert pipe: %s\n",
+ strerror(errno));
+ ast_alertpipe_close(alert_pipe);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+void ast_alertpipe_close(int alert_pipe[2])
+{
+#ifdef HAVE_EVENTFD
+
+ if (alert_pipe[0] == alert_pipe[1]) {
+ if (alert_pipe[0] > -1) {
+ close(alert_pipe[0]);
+ ast_alertpipe_clear(alert_pipe);
+ }
+ return;
+ }
+
+#endif
+
+ if (alert_pipe[0] > -1) {
+ close(alert_pipe[0]);
+ }
+ if (alert_pipe[1] > -1) {
+ close(alert_pipe[1]);
+ }
+ ast_alertpipe_clear(alert_pipe);
+}
+
+ast_alert_status_t ast_alertpipe_read(int alert_pipe[2])
+{
+ uint64_t tmp;
+
+ if (!ast_alertpipe_readable(alert_pipe)) {
+ return AST_ALERT_NOT_READABLE;
+ }
+
+ if (read(alert_pipe[0], &tmp, sizeof(tmp)) < 0) {
+ if (errno != EINTR && errno != EAGAIN) {
+ ast_log(LOG_WARNING, "read() failed: %s\n", strerror(errno));
+ return AST_ALERT_READ_FAIL;
+ }
+ }
+
+ return AST_ALERT_READ_SUCCESS;
+}
+
+ssize_t ast_alertpipe_write(int alert_pipe[2])
+{
+ uint64_t tmp = 1;
+
+ if (!ast_alertpipe_writable(alert_pipe)) {
+ errno = EBADF;
+ return 0;
+ }
+
+ /* preset errno in case returned size does not match */
+ errno = EPIPE;
+ return write(alert_pipe[1], &tmp, sizeof(tmp)) != sizeof(tmp);
+}
+
+ast_alert_status_t ast_alertpipe_flush(int alert_pipe[2])
+{
+ int bytes_read;
+ uint64_t tmp[16];
+
+ if (!ast_alertpipe_readable(alert_pipe)) {
+ return AST_ALERT_NOT_READABLE;
+ }
+
+ /* Read the alertpipe until it is exhausted. */
+ for (;;) {
+ bytes_read = read(alert_pipe[0], tmp, sizeof(tmp));
+ if (bytes_read < 0) {
+ if (errno == EINTR) {
+ continue;
+ }
+ if (errno == EAGAIN || errno == EWOULDBLOCK) {
+ /*
+ * Would block so nothing left to read.
+ * This is the normal loop exit.
+ */
+ break;
+ }
+ ast_log(LOG_WARNING, "read() failed flushing alertpipe: %s\n",
+ strerror(errno));
+ return AST_ALERT_READ_FAIL;
+ }
+ if (!bytes_read) {
+ /* Read nothing so we are done */
+ break;
+ }
+ }
+
+ return AST_ALERT_READ_SUCCESS;
+}
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 652d40c..b466b3c 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -37,6 +37,7 @@
#include <signal.h>
#include "asterisk/heap.h"
+#include "asterisk/alertpipe.h"
#include "asterisk/astobj2.h"
#include "asterisk/stringfields.h"
#include "asterisk/app.h"
@@ -956,7 +957,6 @@
int ast_bridge_channel_queue_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *fr)
{
struct ast_frame *dup;
- char nudge = 0;
if (bridge_channel->suspended
/* Also defer DTMF frames. */
@@ -985,7 +985,7 @@
}
AST_LIST_INSERT_TAIL(&bridge_channel->wr_queue, dup, frame_list);
- if (write(bridge_channel->alert_pipe[1], &nudge, sizeof(nudge)) != sizeof(nudge)) {
+ if (ast_alertpipe_write(bridge_channel->alert_pipe)) {
ast_log(LOG_ERROR, "We couldn't write alert pipe for %p(%s)... something is VERY wrong\n",
bridge_channel, ast_channel_name(bridge_channel->chan));
}
@@ -2259,25 +2259,6 @@
/*!
* \internal
- * \param bridge_channel Channel to read wr_queue alert pipe.
- *
- * \return Nothing
- */
-static void bridge_channel_read_wr_queue_alert(struct ast_bridge_channel *bridge_channel)
-{
- char nudge;
-
- if (read(bridge_channel->alert_pipe[0], &nudge, sizeof(nudge)) < 0) {
- if (errno != EINTR && errno != EAGAIN) {
- ast_log(LOG_WARNING, "read() failed for alert pipe on %p(%s): %s\n",
- bridge_channel, ast_channel_name(bridge_channel->chan),
- strerror(errno));
- }
- }
-}
-
-/*!
- * \internal
* \brief Handle bridge channel write frame to channel.
* \since 12.0.0
*
@@ -2298,7 +2279,7 @@
/* No frame, flush the alert pipe of excess alerts. */
ast_log(LOG_WARNING, "Weird. No frame from bridge for %s to process?\n",
ast_channel_name(bridge_channel->chan));
- bridge_channel_read_wr_queue_alert(bridge_channel);
+ ast_alertpipe_read(bridge_channel->alert_pipe);
ast_bridge_channel_unlock(bridge_channel);
return;
}
@@ -2314,7 +2295,7 @@
break;
}
}
- bridge_channel_read_wr_queue_alert(bridge_channel);
+ ast_alertpipe_read(bridge_channel->alert_pipe);
AST_LIST_REMOVE_CURRENT(frame_list);
break;
}
@@ -2852,62 +2833,6 @@
&& AST_LIST_EMPTY(&bridge_channel->wr_queue);
}
-/*!
- * \internal
- * \brief Close a pipe.
- * \since 12.0.0
- *
- * \param my_pipe What to close.
- *
- * \return Nothing
- */
-static void pipe_close(int *my_pipe)
-{
- if (my_pipe[0] > -1) {
- close(my_pipe[0]);
- my_pipe[0] = -1;
- }
- if (my_pipe[1] > -1) {
- close(my_pipe[1]);
- my_pipe[1] = -1;
- }
-}
-
-/*!
- * \internal
- * \brief Initialize a pipe as non-blocking.
- * \since 12.0.0
- *
- * \param my_pipe What to initialize.
- *
- * \retval 0 on success.
- * \retval -1 on error.
- */
-static int pipe_init_nonblock(int *my_pipe)
-{
- int flags;
-
- my_pipe[0] = -1;
- my_pipe[1] = -1;
- if (pipe(my_pipe)) {
- ast_log(LOG_WARNING, "Can't create pipe! Try increasing max file descriptors with ulimit -n\n");
- return -1;
- }
- flags = fcntl(my_pipe[0], F_GETFL);
- if (fcntl(my_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
- ast_log(LOG_WARNING, "Unable to set read pipe nonblocking! (%d: %s)\n",
- errno, strerror(errno));
- return -1;
- }
- flags = fcntl(my_pipe[1], F_GETFL);
- if (fcntl(my_pipe[1], F_SETFL, flags | O_NONBLOCK) < 0) {
- ast_log(LOG_WARNING, "Unable to set write pipe nonblocking! (%d: %s)\n",
- errno, strerror(errno));
- return -1;
- }
- return 0;
-}
-
/* Destroy elements of the bridge channel structure and the bridge channel structure itself */
static void bridge_channel_destroy(void *obj)
{
@@ -2927,7 +2852,7 @@
while ((fr = AST_LIST_REMOVE_HEAD(&bridge_channel->wr_queue, frame_list))) {
bridge_frame_free(fr);
}
- pipe_close(bridge_channel->alert_pipe);
+ ast_alertpipe_close(bridge_channel->alert_pipe);
ast_cond_destroy(&bridge_channel->cond);
@@ -2944,7 +2869,7 @@
return NULL;
}
ast_cond_init(&bridge_channel->cond, NULL);
- if (pipe_init_nonblock(bridge_channel->alert_pipe)) {
+ if (ast_alertpipe_init(bridge_channel->alert_pipe)) {
ao2_ref(bridge_channel, -1);
return NULL;
}
diff --git a/main/channel_internal_api.c b/main/channel_internal_api.c
index bb33e9f..d181f52 100644
--- a/main/channel_internal_api.c
+++ b/main/channel_internal_api.c
@@ -38,6 +38,7 @@
#include <unistd.h>
#include <fcntl.h>
+#include "asterisk/alertpipe.h"
#include "asterisk/paths.h"
#include "asterisk/channel.h"
#include "asterisk/channel_internal.h"
@@ -1237,152 +1238,52 @@
/* Alertpipe functions */
int ast_channel_alert_write(struct ast_channel *chan)
{
- char blah = 0x7F;
-
- if (!ast_channel_alert_writable(chan)) {
- errno = EBADF;
- return 0;
- }
- /* preset errno in case returned size does not match */
- errno = EPIPE;
- return write(chan->alertpipe[1], &blah, sizeof(blah)) != sizeof(blah);
-}
-
-static int channel_internal_alert_check_nonblock(struct ast_channel *chan)
-{
- int flags;
-
- flags = fcntl(chan->alertpipe[0], F_GETFL);
- /* For some odd reason, the alertpipe occasionally loses nonblocking status,
- * which immediately causes a deadlock scenario. Detect and prevent this. */
- if ((flags & O_NONBLOCK) == 0) {
- ast_log(LOG_ERROR, "Alertpipe on channel %s lost O_NONBLOCK?!!\n", ast_channel_name(chan));
- if (fcntl(chan->alertpipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
- ast_log(LOG_WARNING, "Unable to set alertpipe nonblocking! (%d: %s)\n", errno, strerror(errno));
- return -1;
- }
- }
- return 0;
+ return ast_alertpipe_write(chan->alertpipe);
}
ast_alert_status_t ast_channel_internal_alert_flush(struct ast_channel *chan)
{
- int bytes_read;
- char blah[100];
-
- if (!ast_channel_internal_alert_readable(chan)) {
- return AST_ALERT_NOT_READABLE;
- }
- if (channel_internal_alert_check_nonblock(chan)) {
- return AST_ALERT_READ_FATAL;
- }
-
- /* Read the alertpipe until it is exhausted. */
- for (;;) {
- bytes_read = read(chan->alertpipe[0], blah, sizeof(blah));
- if (bytes_read < 0) {
- if (errno == EINTR) {
- continue;
- }
- if (errno == EAGAIN || errno == EWOULDBLOCK) {
- /*
- * Would block so nothing left to read.
- * This is the normal loop exit.
- */
- break;
- }
- ast_log(LOG_WARNING, "read() failed flushing alertpipe: %s\n",
- strerror(errno));
- return AST_ALERT_READ_FAIL;
- }
- if (!bytes_read) {
- /* Read nothing so we are done */
- break;
- }
- }
-
- return AST_ALERT_READ_SUCCESS;
+ return ast_alertpipe_flush(chan->alertpipe);
}
ast_alert_status_t ast_channel_internal_alert_read(struct ast_channel *chan)
{
- char blah;
-
- if (!ast_channel_internal_alert_readable(chan)) {
- return AST_ALERT_NOT_READABLE;
- }
- if (channel_internal_alert_check_nonblock(chan)) {
- return AST_ALERT_READ_FATAL;
- }
-
- if (read(chan->alertpipe[0], &blah, sizeof(blah)) < 0) {
- if (errno != EINTR && errno != EAGAIN) {
- ast_log(LOG_WARNING, "read() failed: %s\n", strerror(errno));
- return AST_ALERT_READ_FAIL;
- }
- }
-
- return AST_ALERT_READ_SUCCESS;
+ return ast_alertpipe_read(chan->alertpipe);
}
int ast_channel_alert_writable(struct ast_channel *chan)
{
- return chan->alertpipe[1] > -1;
+ return ast_alertpipe_writable(chan->alertpipe);
}
int ast_channel_internal_alert_readable(struct ast_channel *chan)
{
- return chan->alertpipe[0] > -1;
+ return ast_alertpipe_readable(chan->alertpipe);
}
void ast_channel_internal_alertpipe_clear(struct ast_channel *chan)
{
- chan->alertpipe[0] = chan->alertpipe[1] = -1;
+ ast_alertpipe_clear(chan->alertpipe);
}
void ast_channel_internal_alertpipe_close(struct ast_channel *chan)
{
- if (ast_channel_internal_alert_readable(chan)) {
- close(chan->alertpipe[0]);
- chan->alertpipe[0] = -1;
- }
- if (ast_channel_alert_writable(chan)) {
- close(chan->alertpipe[1]);
- chan->alertpipe[1] = -1;
- }
+ ast_alertpipe_close(chan->alertpipe);
}
int ast_channel_internal_alertpipe_init(struct ast_channel *chan)
{
- if (pipe(chan->alertpipe)) {
- ast_log(LOG_WARNING, "Channel allocation failed: Can't create alert pipe! Try increasing max file descriptors with ulimit -n\n");
- return -1;
- } else {
- int flags = fcntl(chan->alertpipe[0], F_GETFL);
- if (fcntl(chan->alertpipe[0], F_SETFL, flags | O_NONBLOCK) < 0) {
- ast_log(LOG_WARNING, "Channel allocation failed: Unable to set alertpipe nonblocking! (%d: %s)\n", errno, strerror(errno));
- return -1;
- }
- flags = fcntl(chan->alertpipe[1], F_GETFL);
- if (fcntl(chan->alertpipe[1], F_SETFL, flags | O_NONBLOCK) < 0) {
- ast_log(LOG_WARNING, "Channel allocation failed: Unable to set alertpipe nonblocking! (%d: %s)\n", errno, strerror(errno));
- return -1;
- }
- }
- return 0;
+ return ast_alertpipe_init(chan->alertpipe);
}
int ast_channel_internal_alert_readfd(struct ast_channel *chan)
{
- return chan->alertpipe[0];
+ return ast_alertpipe_readfd(chan->alertpipe);
}
void ast_channel_internal_alertpipe_swap(struct ast_channel *chan1, struct ast_channel *chan2)
{
- int i;
- for (i = 0; i < ARRAY_LEN(chan1->alertpipe); i++) {
- SWAP(chan1->alertpipe[i], chan2->alertpipe[i]);
- }
+ ast_alertpipe_swap(chan1->alertpipe, chan2->alertpipe);
}
/* file descriptor array accessors */
diff --git a/main/utils.c b/main/utils.c
index de7ff8f..b31db59 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -69,6 +69,9 @@
#define AST_API_MODULE
#include "asterisk/config.h"
+#define AST_API_MODULE
+#include "asterisk/alertpipe.h"
+
static char base64[64];
static char b2a[256];
--
To view, visit https://gerrit.asterisk.org/5497
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iff0fb1573e7f7a187d5211ddc60aa8f3da3edb1d
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
More information about the asterisk-commits
mailing list