Commit a3f53566 authored by Fabrice Bellet's avatar Fabrice Bellet Committed by Olivier Crête

agent: don't allocate large arrays on the stack

parent b353f30c
...@@ -75,10 +75,6 @@ ...@@ -75,10 +75,6 @@
#include "pseudotcp.h" #include "pseudotcp.h"
#include "agent-enum-types.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_STUN_PORT 3478
#define DEFAULT_UPNP_TIMEOUT 200 /* milliseconds */ #define DEFAULT_UPNP_TIMEOUT 200 /* milliseconds */
#define DEFAULT_IDLE_TIMEOUT 5000 /* milliseconds */ #define DEFAULT_IDLE_TIMEOUT 5000 /* milliseconds */
...@@ -1967,12 +1963,12 @@ pseudo_tcp_socket_readable (PseudoTcpSocket *sock, gpointer user_data) ...@@ -1967,12 +1963,12 @@ pseudo_tcp_socket_readable (PseudoTcpSocket *sock, gpointer user_data)
* no data loss of packets already received and dequeued. */ * no data loss of packets already received and dequeued. */
if (has_io_callback) { if (has_io_callback) {
do { do {
guint8 buf[MAX_BUFFER_SIZE];
gssize len; gssize len;
/* FIXME: Why copy into a temporary buffer here? Why can’t the I/O /* FIXME: Why copy into a temporary buffer here? Why can’t the I/O
* callbacks be emitted directly from the pseudo-TCP receive buffer? */ * 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", nice_debug ("%s: I/O callback case: Received %" G_GSSIZE_FORMAT " bytes",
G_STRFUNC, len); G_STRFUNC, len);
...@@ -2006,7 +2002,7 @@ pseudo_tcp_socket_readable (PseudoTcpSocket *sock, gpointer user_data) ...@@ -2006,7 +2002,7 @@ pseudo_tcp_socket_readable (PseudoTcpSocket *sock, gpointer user_data)
break; 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, if (!agent_find_component (agent, stream_id, component_id,
&stream, &component)) { &stream, &component)) {
...@@ -5748,10 +5744,9 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data) ...@@ -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 * 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 * memcpy()s can be achieved (for in-order packet delivery) by emittin the
* I/O callback directly from the pseudo-TCP receive buffer. */ * I/O callback directly from the pseudo-TCP receive buffer. */
guint8 local_body_buf[MAX_BUFFER_SIZE];
GInputVector local_bufs[] = { GInputVector local_bufs[] = {
{ local_header_buf, sizeof (local_header_buf) }, { local_header_buf, sizeof (local_header_buf) },
{ local_body_buf, sizeof (local_body_buf) }, { component->recv_buffer, component->recv_buffer_size },
}; };
NiceInputMessage local_message = { NiceInputMessage local_message = {
local_bufs, G_N_ELEMENTS (local_bufs), NULL, 0 local_bufs, G_N_ELEMENTS (local_bufs), NULL, 0
...@@ -5798,8 +5793,9 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data) ...@@ -5798,8 +5793,9 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data)
} }
} else if (has_io_callback) { } else if (has_io_callback) {
while (has_io_callback) { while (has_io_callback) {
guint8 local_buf[MAX_BUFFER_SIZE]; GInputVector local_bufs = {
GInputVector local_bufs = { local_buf, sizeof (local_buf) }; component->recv_buffer, component->recv_buffer_size
};
NiceInputMessage local_message = { &local_bufs, 1, NULL, 0 }; NiceInputMessage local_message = { &local_bufs, 1, NULL, 0 };
RecvStatus retval; RecvStatus retval;
...@@ -5824,7 +5820,7 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data) ...@@ -5824,7 +5820,7 @@ component_io_cb (GSocket *gsocket, GIOCondition condition, gpointer user_data)
" bytes", G_STRFUNC, agent, local_message.length); " bytes", G_STRFUNC, agent, local_message.length);
if (local_message.length > 0) { if (local_message.length > 0) {
nice_component_emit_io_callback (agent, component, local_buf, nice_component_emit_io_callback (agent, component,
local_message.length); local_message.length);
} }
} }
......
...@@ -386,6 +386,8 @@ nice_component_close (NiceAgent *agent, NiceStream *stream, NiceComponent *cmp) ...@@ -386,6 +386,8 @@ nice_component_close (NiceAgent *agent, NiceStream *stream, NiceComponent *cmp)
g_free ((gpointer) vec->buffer); g_free ((gpointer) vec->buffer);
g_slice_free (GOutputVector, vec); g_slice_free (GOutputVector, vec);
} }
g_free (cmp->recv_buffer);
} }
/* /*
...@@ -921,14 +923,13 @@ emit_io_callback_cb (gpointer user_data) ...@@ -921,14 +923,13 @@ emit_io_callback_cb (gpointer user_data)
/* This must be called with the agent lock *held*. */ /* This must be called with the agent lock *held*. */
void void
nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component, nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component,
const guint8 *buf, gsize buf_len) gsize buf_len)
{ {
guint stream_id, component_id; guint stream_id, component_id;
NiceAgentRecvFunc io_callback; NiceAgentRecvFunc io_callback;
gpointer io_user_data; gpointer io_user_data;
g_assert (component != NULL); g_assert (component != NULL);
g_assert (buf != NULL);
g_assert (buf_len > 0); g_assert (buf_len > 0);
stream_id = component->stream_id; stream_id = component->stream_id;
...@@ -955,7 +956,7 @@ nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component, ...@@ -955,7 +956,7 @@ nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component,
/* Thread owns the main context, so invoke the callback directly. */ /* Thread owns the main context, so invoke the callback directly. */
agent_unlock_and_emit (agent); agent_unlock_and_emit (agent);
io_callback (agent, stream_id, 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); agent_lock (agent);
} else { } else {
IOCallbackData *data; IOCallbackData *data;
...@@ -964,7 +965,7 @@ nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component, ...@@ -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 /* Slow path: Current thread doesn’t own the Component’s context at the
* moment, so schedule the callback in an idle handler. */ * 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, g_queue_push_tail (&component->pending_io_messages,
data); /* transfer ownership */ data); /* transfer ownership */
...@@ -1114,6 +1115,13 @@ nice_component_init (NiceComponent *component) ...@@ -1114,6 +1115,13 @@ nice_component_init (NiceComponent *component)
g_queue_init (&component->incoming_checks); g_queue_init (&component->incoming_checks);
component->have_local_consent = TRUE; 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 static void
......
...@@ -233,6 +233,14 @@ struct _NiceComponent { ...@@ -233,6 +233,14 @@ struct _NiceComponent {
GQueue queued_tcp_packets; GQueue queued_tcp_packets;
gboolean have_local_consent; 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 { typedef struct {
...@@ -295,7 +303,7 @@ nice_component_set_io_callback (NiceComponent *component, ...@@ -295,7 +303,7 @@ nice_component_set_io_callback (NiceComponent *component,
GError **error); GError **error);
void void
nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component, nice_component_emit_io_callback (NiceAgent *agent, NiceComponent *component,
const guint8 *buf, gsize buf_len); gsize buf_len);
gboolean gboolean
nice_component_has_io_callback (NiceComponent *component); nice_component_has_io_callback (NiceComponent *component);
void void
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment