agent: don't allocate large arrays on the stack

diff --git a/agent/agent.c b/agent/agent.c
index 1756734..2b9163e 100644 (file)
--- a/agent/agent.c
+++ b/agent/agent.c
@@ -75,10 +75,6 @@
 #include "pseudotcp.h"
 #include "agent-enum-types.h"
 
-/* Maximum size of a UDP packet’s payload, as the packet’s length field is 16b
- * wide. */
-#define MAX_BUFFER_SIZE ((1 << 16) - 1)  /* 65535 */
-
 #define DEFAULT_STUN_PORT  3478
 #define DEFAULT_UPNP_TIMEOUT 200  /* milliseconds */
 #define DEFAULT_IDLE_TIMEOUT 5000 /* milliseconds */
@@ -1967,12 +1963,12 @@ pseudo_tcp_socket_readable (PseudoTcpSocket *sock, gpointer user_data)
    * no data loss of packets already received and dequeued. */
   if (has_io_callback) {
     do {
-      guint8 buf[MAX_BUFFER_SIZE];
       gssize len;
 
       /* FIXME: Why copy into a temporary buffer here? Why can’t the I/O
        * callbacks be emitted directly from the pseudo-TCP receive buffer? */
-      len = pseudo_tcp_socket_recv (sock, (gchar *) buf, sizeof(buf));
+      len = pseudo_tcp_socket_recv (sock, (gchar *) component->recv_buffer,
+          component->recv_buffer_size);
 
       nice_debug ("%s: I/O callback case: Received %" G_GSSIZE_FORMAT " bytes",
           G_STRFUNC, len);
@@ -2006,7 +2002,7 @@ pseudo_tcp_socket_readable (PseudoTcpSocket *sock, gpointer user_data)
         break;
       }
 
-      nice_component_emit_io_callback (agent, component, buf, len);
+      nice_component_emit_io_callback (agent, component, len);
 
       if (!agent_find_component (agent, stream_id, component_id,
               &stream, &component)) {
@@ -5748,10 +5744,9 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data)
      * In fact, in the case of a reliable agent with I/O callbacks, zero
      * memcpy()s can be achieved (for in-order packet delivery) by emittin the
      * I/O callback directly from the pseudo-TCP receive buffer. */
-    guint8 local_body_buf[MAX_BUFFER_SIZE];
     GInputVector local_bufs[] = {
       { local_header_buf, sizeof (local_header_buf) },
-      { local_body_buf, sizeof (local_body_buf) },
+      { component->recv_buffer, component->recv_buffer_size },
     };
     NiceInputMessage local_message = {
       local_bufs, G_N_ELEMENTS (local_bufs), NULL, 0
@@ -5798,8 +5793,9 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data)
     }
   } else if (has_io_callback) {
     while (has_io_callback) {
-      guint8 local_buf[MAX_BUFFER_SIZE];
-      GInputVector local_bufs = { local_buf, sizeof (local_buf) };
+      GInputVector local_bufs = {
+        component->recv_buffer, component->recv_buffer_size
+      };
       NiceInputMessage local_message = { &local_bufs, 1, NULL, 0 };
       RecvStatus retval;
 
@@ -5824,7 +5820,7 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data)
             " bytes", G_STRFUNC, agent, local_message.length);
 
         if (local_message.length > 0) {
-          nice_component_emit_io_callback (agent, component, local_buf,
+          nice_component_emit_io_callback (agent, component,
               local_message.length);
         }
       }

diff --git a/agent/component.c b/agent/component.c
index 2006c42..91ded03 100644 (file)
--- a/agent/component.c
+++ b/agent/component.c
@@ -386,6 +386,8 @@ nice_component_close (NiceAgent *agent, NiceStream *stream, NiceComponent *cmp)
     g_free ((gpointer) vec->buffer);
     g_slice_free (GOutputVector, vec);
   }
+
+  g_free (cmp->recv_buffer);
 }
 
 /*
@@ -921,14 +923,13 @@ emit_io_callback_cb (gpointer user_data)
 /* This must be called with the agent lock *held*. */
 void
 nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component,
-    const guint8 *buf, gsize buf_len)
+    gsize buf_len)
 {
   guint stream_id, component_id;
   NiceAgentRecvFunc io_callback;
   gpointer io_user_data;
 
   g_assert (component != NULL);
-  g_assert (buf != NULL);
   g_assert (buf_len > 0);
 
   stream_id = component->stream_id;
@@ -955,7 +956,7 @@ nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component,
     /* Thread owns the main context, so invoke the callback directly. */
     agent_unlock_and_emit (agent);
     io_callback (agent, stream_id,
-        component_id, buf_len, (gchar *) buf, io_user_data);
+        component_id, buf_len, (gchar *) component->recv_buffer, io_user_data);
     agent_lock (agent);
   } else {
     IOCallbackData *data;
@@ -964,7 +965,7 @@ nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component,
 
     /* Slow path: Current thread doesn’t own the Component’s context at the
      * moment, so schedule the callback in an idle handler. */
-    data = io_callback_data_new (buf, buf_len);
+    data = io_callback_data_new (component->recv_buffer, buf_len);
     g_queue_push_tail (&component->pending_io_messages,
         data);  /* transfer ownership */
 
@@ -1114,6 +1115,13 @@ nice_component_init (NiceComponent *component)
   g_queue_init (&component->incoming_checks);
 
   component->have_local_consent = TRUE;
+
+/* Maximum size of a UDP packet’s payload, as the packet’s length field is 16b
+ * wide. */
+#define MAX_BUFFER_SIZE ((1 << 16) - 1)  /* 65535 */
+
+  component->recv_buffer = g_malloc (MAX_BUFFER_SIZE);
+  component->recv_buffer_size = MAX_BUFFER_SIZE;
 }
 
 static void

diff --git a/agent/component.h b/agent/component.h
index 788e539..aa4346f 100644 (file)
--- a/agent/component.h
+++ b/agent/component.h
@@ -233,6 +233,14 @@ struct _NiceComponent {
   GQueue queued_tcp_packets;
 
   gboolean have_local_consent;
+
+  /* scratch buffer for use in the component_io_cb() function to
+   * hold the incoming packet, allocated at component creation, since
+   * the callback is a critical path, where memory allocation should
+   * be avoided.
+   */
+  guint8 *recv_buffer;
+  guint recv_buffer_size;
 };
 
 typedef struct {
@@ -295,7 +303,7 @@ nice_component_set_io_callback (NiceComponent *component,
     GError **error);
 void
 nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component,
-    const guint8 *buf, gsize buf_len);
+    gsize buf_len);
 gboolean
 nice_component_has_io_callback (NiceComponent *component);
 void