]> git.ipfire.org Git - thirdparty/freeswitch.git/commitdiff
[mod_erlang_event] Fix correctness, OTP compatibility, and memory issues
authorDmitry Verenitsin <morbit85@gmail.com>
Mon, 25 May 2026 21:12:08 +0000 (02:12 +0500)
committerGitHub <noreply@github.com>
Mon, 25 May 2026 21:12:08 +0000 (00:12 +0300)
Changes:
- Snapshot `erl_errno` after `ei_xreceive_msg_tmo()` — outbound `ei_*` calls in the same loop iteration clobber the thread-local errno before the listener checks it, causing wrong exit decisions and misleading logs.
- Fix `switch_size_t ` cast of `int` in `ei_link`* — `(switch_size_t *)&index` reads/writes 8 bytes through a 4-byte `int` on LP64. Use a real `switch_size_t` local.
- Dispatch `ERL_NEWER_REFERENCE_EXT` — newer OTP encodes refs with this tag; spawn replies from modern nodes were silently dropped to the default branch.
- Handle `ERL_EXIT2` — processes killed via `erlang:exit/2` arrive with this tag, not `ERL_EXIT`. Without it, sessions stayed attached to dead Erlang pids.
- Modernize `-spec` syntax in `freeswitch.erl` — old `-spec(F/N :: (...))` form was removed in OTP 21+; module no longer compiled.
- Fix multiple memory issues:
  - `ei_hash_ref()`: replace unbounded `sprintf` with `snprintf` + shared `EI_HASH_REF_LEN`.
  - `handle_msg_sendevent` / `handle_msg_sendmsg`: free the heap `value` on `ei_decode_string` failure; remove dead `if (!fail)` branches.
  - `listener_main_loop`: free `buf`/`rbuf` on the two `handle_msg` early-exit paths.
  - `erlang_sendmsg_function` app: move `ei_x_new_with_version` past arg validation and add `ei_x_free` at the end.

src/mod/event_handlers/mod_erlang_event/ei_helpers.c
src/mod/event_handlers/mod_erlang_event/freeswitch.erl
src/mod/event_handlers/mod_erlang_event/handle_msg.c
src/mod/event_handlers/mod_erlang_event/mod_erlang_event.c
src/mod/event_handlers/mod_erlang_event/mod_erlang_event.h

index 6472180a2eda5ac1ac0ec51d1baa49243d1e2e75..74dc860cb16d7cf4b5bd2ccd88fed653fedf9dc2 100644 (file)
@@ -64,6 +64,7 @@ void ei_link(listener_t *listener, erlang_pid * from, erlang_pid * to)
        char msgbuf[2048];
        char *s;
        int index = 0;
+       switch_size_t send_len;
        int status = SWITCH_STATUS_SUCCESS;
        switch_socket_t *sock = NULL;
        switch_os_sock_put(&sock, &listener->sockdes, listener->pool);
@@ -82,7 +83,8 @@ void ei_link(listener_t *listener, erlang_pid * from, erlang_pid * to)
        /* sum:  542 */
 
        switch_mutex_lock(listener->sock_mutex);
-       status = switch_socket_send(sock, msgbuf, (switch_size_t *) &index);
+       send_len = (switch_size_t)index;
+       status = switch_socket_send(sock, msgbuf, &send_len);
        if (status != SWITCH_STATUS_SUCCESS) {
                switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_WARNING, "Failed to link to process on %s\n", listener->peer_nodename);
        }
@@ -283,8 +285,7 @@ int ei_sendto(ei_cnode * ec, int fd, struct erlang_process *process, ei_x_buff *
 /* convert an erlang reference to some kind of hashed string so we can store it as a hash key */
 void ei_hash_ref(erlang_ref * ref, char *output)
 {
-       /* very lazy */
-       sprintf(output, "%d.%d.%d@%s", ref->n[0], ref->n[1], ref->n[2], ref->node);
+       snprintf(output, EI_HASH_REF_LEN, "%d.%d.%d@%s", ref->n[0], ref->n[1], ref->n[2], ref->node);
 }
 
 
index 9a08d88ae2c2d3d73b29d377cfd512855812bb8a..a82f53b6514848c2f7b959f28028124018caab88 100644 (file)
@@ -91,7 +91,7 @@ api(Node, Cmd) ->
 %% sent to calling process after it is received. This function
 %% returns the result of the initial bgapi call or `timeout' if FreeSWITCH fails
 %% to respond.
--spec(bgapi/3 :: (Node :: atom(), Cmd :: atom(), Args :: string()) -> {'ok', string()} | {'error', any()} | 'timeout').
+-spec bgapi(Node :: atom(), Cmd :: atom(), Args :: string()) -> {'ok', string()} | {'error', any()} | 'timeout'.
 bgapi(Node, Cmd, Args) ->
        Self = self(),
        % spawn a new process so that both responses go here instead of directly to
@@ -128,7 +128,7 @@ bgapi(Node, Cmd, Args) ->
 %% passed as the argument to `Fun' after it is received. This function
 %% returns the result of the initial bgapi call or `timeout' if FreeSWITCH fails
 %% to respond.
--spec(bgapi/4 :: (Node :: atom(), Cmd :: atom(), Args :: string(), Fun :: fun()) -> 'ok' | {'error', any()} | 'timeout').
+-spec bgapi(Node :: atom(), Cmd :: atom(), Args :: string(), Fun :: fun()) -> 'ok' | {'error', any()} | 'timeout'.
 bgapi(Node, Cmd, Args, Fun) ->
        Self = self(),
        % spawn a new process so that both responses go here instead of directly to
index aad45a4986114ae9bfaf964d061fc8f6c3630422..a54a5123e98ce1caacccded8c9e2a31d5e5a374c 100644 (file)
@@ -804,13 +804,13 @@ static switch_status_t handle_msg_sendevent(listener_t *listener, int arity, ei_
        } else {
                switch_event_types_t etype;
                if (switch_name_event(ename, &etype) == SWITCH_STATUS_SUCCESS) {
-                       switch_event_t *event;
+                       switch_event_t *event = NULL;
                        if ((strlen(esname) && switch_event_create_subclass(&event, etype, esname) == SWITCH_STATUS_SUCCESS) ||
                                switch_event_create(&event, etype) == SWITCH_STATUS_SUCCESS) {
                                char key[1024];
-                               char *value;
-                                int type;
-                                int size;
+                               char *value = NULL;
+                               int type;
+                               int size;
                                int i = 0;
                                switch_bool_t fail = SWITCH_FALSE;
 
@@ -828,14 +828,15 @@ static switch_status_t handle_msg_sendevent(listener_t *listener, int arity, ei_
                                        value = malloc(size + 1);
 
                                        if (ei_decode_string(buf->buff, &buf->index, value)) {
-                                                       fail = SWITCH_TRUE;
+                                               switch_safe_free(value);
+                                               fail = SWITCH_TRUE;
                                                break;
                                        }
 
-                                       if (!fail && !strcmp(key, "body")) {
+                                       if (!strcmp(key, "body")) {
                                                switch_safe_free(event->body);
                                                event->body = value;
-                                       } else if (!fail)  {
+                                       } else {
                                                switch_event_add_header_string_nodup(event, SWITCH_STACK_BOTTOM, key, value);
                                        }
 
@@ -896,13 +897,12 @@ static switch_status_t handle_msg_sendmsg(listener_t *listener, int arity, ei_x_
                                        value = malloc(size + 1);
 
                                        if (ei_decode_string(buf->buff, &buf->index, value)) {
+                                               switch_safe_free(value);
                                                fail = SWITCH_TRUE;
                                                break;
                                        }
 
-                                       if (!fail) {
-                                               switch_event_add_header_string_nodup(event, SWITCH_STACK_BOTTOM, key, value);
-                                       }
+                                       switch_event_add_header_string_nodup(event, SWITCH_STACK_BOTTOM, key, value);
                                }
 
                                if (headerlength != i || fail) {
@@ -1204,7 +1204,7 @@ static switch_status_t handle_ref_tuple(listener_t *listener, erlang_msg * msg,
 {
        erlang_ref ref;
        erlang_pid pid;
-       char hash[100];
+       char hash[EI_HASH_REF_LEN];
        int arity;
        const void *key;
        void *val;
@@ -1232,7 +1232,7 @@ static switch_status_t handle_ref_tuple(listener_t *listener, erlang_msg * msg,
        for (iter = switch_core_hash_first(listener->sessions); iter; iter = switch_core_hash_next(&iter)) {
                switch_core_hash_this(iter, &key, NULL, &val);
                se = (session_elem_t*)val;
-               if (switch_test_flag(se, LFLAG_WAITING_FOR_PID) && se->spawn_reply && !strncmp(se->spawn_reply->hash, hash, 100)) {
+               if (switch_test_flag(se, LFLAG_WAITING_FOR_PID) && se->spawn_reply && !strncmp(se->spawn_reply->hash, hash, EI_HASH_REF_LEN)) {
 
                        switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "found matching session for %s : %s\n", hash, se->uuid_str);
 
@@ -1376,6 +1376,7 @@ int handle_msg(listener_t *listener, erlang_msg * msg, ei_x_buff * buf, ei_x_buf
                                break;
                        case ERL_REFERENCE_EXT:
                        case ERL_NEW_REFERENCE_EXT:
+                       case ERL_NEWER_REFERENCE_EXT:
                                ret = handle_ref_tuple(listener, msg, buf, rbuf);
                                break;
                        default:
index f163b2197f09f24a969bbbc315516808d93c407c..3315d60fb84abc5650b695f13b7bc1c72b3e5b9c 100644 (file)
@@ -968,9 +968,10 @@ static void handle_exit(listener_t *listener, erlang_pid * pid)
 static void listener_main_loop(listener_t *listener)
 {
        int status = 1;
+       int recv_erl_errno = ETIMEDOUT;
        int msgs_sent = 0; /* how many messages we sent in a loop */
 
-       while ((status >= 0 || erl_errno == ETIMEDOUT || erl_errno == EAGAIN) && !prefs.done) {
+       while ((status >= 0 || recv_erl_errno == ETIMEDOUT || recv_erl_errno == EAGAIN) && !prefs.done) {
                erlang_msg msg;
                ei_x_buff buf;
                ei_x_buff rbuf;
@@ -983,6 +984,9 @@ static void listener_main_loop(listener_t *listener)
                /* do we need the mutex when reading? */
                /*switch_mutex_lock(listener->sock_mutex); */
                status = ei_xreceive_msg_tmo(listener->sockdes, &msg, &buf, 1);
+               /* snapshot erl_errno before any outbound ei call (queue flushers below)
+                  clobbers this thread-local slot. */
+               recv_erl_errno = erl_errno;
                /*switch_mutex_unlock(listener->sock_mutex); */
 
                switch (status) {
@@ -1001,6 +1005,8 @@ static void listener_main_loop(listener_t *listener)
 
                                if (handle_msg(listener, &msg, &buf, &rbuf)) {
                                        switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "handle_msg requested exit\n");
+                                       ei_x_free(&buf);
+                                       ei_x_free(&rbuf);
                                        return;
                                }
                                break;
@@ -1016,6 +1022,8 @@ static void listener_main_loop(listener_t *listener)
 
                                if (handle_msg(listener, &msg, &buf, &rbuf)) {
                                        switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "handle_msg requested exit\n");
+                                       ei_x_free(&buf);
+                                       ei_x_free(&rbuf);
                                        return;
                                }
                                break;
@@ -1026,6 +1034,7 @@ static void listener_main_loop(listener_t *listener)
                                switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "erl_unlink\n");
                                break;
                        case ERL_EXIT:
+                       case ERL_EXIT2:
                                switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "erl_exit from %s <%d.%d.%d>\n", msg.from.node, msg.from.creation, msg.from.num,
                                                                  msg.from.serial);
 
@@ -1037,8 +1046,8 @@ static void listener_main_loop(listener_t *listener)
                        }
                        break;
                case ERL_ERROR:
-                       if (erl_errno != ETIMEDOUT && erl_errno != EAGAIN) {
-                               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "erl_error: status=%d, erl_errno=%d errno=%d\n", status,  erl_errno, errno);
+                       if (recv_erl_errno != ETIMEDOUT && recv_erl_errno != EAGAIN) {
+                               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "erl_error: status=%d, erl_errno=%d errno=%d\n", status,  recv_erl_errno, errno);
                        }
                        break;
                default:
@@ -1069,7 +1078,7 @@ static void listener_main_loop(listener_t *listener)
        if (prefs.done) {
                switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "shutting down listener\n");
        } else {
-               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "listener exit: status=%d, erl_errno=%d errno=%d\n", status,  erl_errno, errno);
+               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "listener exit: status=%d, erl_errno=%d errno=%d\n", status,  recv_erl_errno, errno);
        }
 }
 
@@ -1513,7 +1522,7 @@ session_elem_t *attach_call_to_spawned_process(listener_t *listener, char *modul
 {
        /* create a session list element */
        session_elem_t *session_element = session_elem_create(listener, session);
-       char hash[100];
+       char hash[EI_HASH_REF_LEN];
        spawn_reply_t *p;
        erlang_ref ref;
 
@@ -1720,8 +1729,6 @@ SWITCH_STANDARD_APP(erlang_sendmsg_function)
        ei_x_buff buf;
        listener_t *listener;
 
-       ei_x_new_with_version(&buf);
-
        /* process app arguments */
        if (data && (mydata = switch_core_session_strdup(session, data))) {
                argc = switch_separate_string(mydata, ' ', argv, 3);
@@ -1737,6 +1744,7 @@ SWITCH_STANDARD_APP(erlang_sendmsg_function)
 
        /*switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "sendmsg: {%s, %s} ! %s\n", reg_name, node, argv[2]); */
 
+       ei_x_new_with_version(&buf);
        ei_x_encode_tuple_header(&buf, 2);
        ei_x_encode_atom(&buf, "freeswitch_sendmsg");
        _ei_x_encode_string(&buf, argv[2]);
@@ -1754,6 +1762,8 @@ SWITCH_STANDARD_APP(erlang_sendmsg_function)
 
                switch_thread_rwlock_unlock(listener->rwlock);
        }
+
+       ei_x_free(&buf);
 }
 
 
index 2f2c0ed0591001affdaf1838dd28799be55f9f34..e662273208d59953724e2079b57ef701da24484e 100644 (file)
@@ -238,6 +238,7 @@ extern prefs_t prefs;
 int handle_msg(listener_t *listener, erlang_msg * msg, ei_x_buff * buf, ei_x_buff * rbuf);
 
 /* ei_helpers.c */
+#define EI_HASH_REF_LEN (MAXATOMLEN_UTF8 + 64)
 void ei_link(listener_t *listener, erlang_pid * from, erlang_pid * to);
 void ei_encode_switch_event_headers(ei_x_buff * ebuf, switch_event_t *event);
 void ei_encode_switch_event_tag(ei_x_buff * ebuf, switch_event_t *event, char *tag);