Fix memory leak in JitterBufferHEADmaster

This leak appears only when using a custom destroy callback (set via the
JITTER_BUFFER_SET_DESTROY_CALLBACK ctl message).

In order to understand how this happens, the following information needs
to be known:
- The jitter buffer implementation always overwrites a freed pointer
  with NULL and therefore can check for NULL in order to determine if a
  given packet's data buffer has been freed already.
- When accessing the data in the buffer from the outside, you'll get a
  copy of the data when NOT using a custom destroy callback but simply a
  copy of the pointer to the original buffer, when using a custom destroy
  callback.

Because of the latter, the query methods can't immediately free the
original data buffer if a custom delete callback is used, as the
returned pointer has to remain valid. In the other case (no custom
callback), the data is freed immediately after it has been copied.

The issue arises, because the data-pointer in the internal packet
structure is UNCONDITIONALLY (aka: regardless of whether a custom
callback is used or not) overwritten by NULL, indicating to all
following code that the data buffer has been freed already. However, as
described above this is not the case, if a custom destroy callback is
used.
Therefore, the data buffer will never be freed in that case (unless the
calling code deletes the data itself. However, as there is no
documentation that implies otherwise, it seems a fair assumption on the
caller's site that the buffer will handle the lifetime of the data
buffer even if a custom destroy callback has been registered. Therefore,
the calling code wouldn't actually perform the freeing) causing a memory
leak.

This commit fixes this issue simply by making sure to not overwrite the
data pointer with NULL, in case a custom destroy callback is used.
Therefore, the data pointer is only set to NULL, when it is actually
freed and otherwise the follow-up code inside the jitter buffer will
know that the buffer hasn't been freed yet and can act accordingly.

1 files changed, 2 insertions, 2 deletions

diff --git a/libspeexdsp/jitter.c b/libspeexdsp/jitter.c
index a9c0dd3..a2a301e 100644
--- a/libspeexdsp/jitter.c
+++ b/libspeexdsp/jitter.c
@@ -603,8 +603,8 @@ EXPORT int jitter_buffer_get(JitterBuffer *jitter, JitterBufferPacket *packet, s
             packet->data[j] = jitter->packets[i].data[j];
          /* Remove packet */
          speex_free(jitter->packets[i].data);
+         jitter->packets[i].data = NULL;
       }
-      jitter->packets[i].data = NULL;
       /* Set timestamp and span (if requested) */
       offset = (spx_int32_t)jitter->packets[i].timestamp-(spx_int32_t)jitter->pointer_timestamp;
       if (start_offset != NULL)
@@ -693,8 +693,8 @@ EXPORT int jitter_buffer_get_another(JitterBuffer *jitter, JitterBufferPacket *p
             packet->data[j] = jitter->packets[i].data[j];
          /* Remove packet */
          speex_free(jitter->packets[i].data);
+         jitter->packets[i].data = NULL;
       }
-      jitter->packets[i].data = NULL;
       packet->timestamp = jitter->packets[i].timestamp;
       packet->span = jitter->packets[i].span;
       packet->sequence = jitter->packets[i].sequence;