* some iochannel fixes

* introduce reference counting in ioline
* fix memory leak in socket-client.c
* fix double-free error in protocol-esound.c


git-svn-id: file:///home/lennart/svn/public/pulseaudio/trunk@293 fefdeb5f-60dc-0310-8127-8f9354f1896f

polyp/Makefile.am

@@ -215,8 +215,7 @@ polypaudio_SOURCES = idxset.c idxset.h \
 polypaudio_CFLAGS = $(AM_CFLAGS) $(LIBSAMPLERATE_CFLAGS) $(LIBSNDFILE_CFLAGS)
 polypaudio_INCLUDES = $(INCLTDL)
 polypaudio_LDADD = $(AM_LDADD) $(LIBLTDL) $(LIBSAMPLERATE_LIBS) $(LIBSNDFILE_LIBS) $(CAP_LIBS)
-polypaudio_LDFLAGS= -export-dynamic -dlopen force
+polypaudio_LDFLAGS= -export-dynamic -dlopen force #-static $(foreach f,$(modlib_LTLIBRARIES),-dlpreopen $(f))
-# -static $(foreach f,$(modlib_LTLIBRARIES),-dlpreopen $(f))
 
 libprotocol_simple_la_SOURCES = protocol-simple.c protocol-simple.h
 libprotocol_simple_la_LDFLAGS = -avoid-version

polyp/cli.c

@@ -92,7 +92,8 @@ struct pa_cli* pa_cli_new(struct pa_core *core, struct pa_iochannel *io, struct
 
 void pa_cli_free(struct pa_cli *c) {
     assert(c);
-    pa_ioline_free(c->line);
+    pa_ioline_close(c->line);
+    pa_ioline_unref(c->line);
     pa_client_free(c->client);
     pa_xfree(c);
 }

polyp/iochannel.c

@@ -235,3 +235,8 @@ int pa_iochannel_socket_set_sndbuf(struct pa_iochannel *io, size_t l) {
     return pa_socket_set_sndbuf(io->ofd, l);
 }
 
+
+struct pa_mainloop_api* pa_iochannel_get_mainloop_api(struct pa_iochannel *io) {
+    assert(io);
+    return io->mainloop;
+}

polyp/iochannel.h

@@ -47,4 +47,6 @@ void pa_iochannel_socket_peer_to_string(struct pa_iochannel*io, char*s, size_t l
 int pa_iochannel_socket_set_rcvbuf(struct pa_iochannel*io, size_t l);
 int pa_iochannel_socket_set_sndbuf(struct pa_iochannel*io, size_t l);
 
+struct pa_mainloop_api* pa_iochannel_get_mainloop_api(struct pa_iochannel *io);
+
 #endif

polyp/ioline.c

@@ -31,12 +31,16 @@
 
 #include "ioline.h"
 #include "xmalloc.h"
+#include "log.h"
 
 #define BUFFER_LIMIT (64*1024)
 #define READ_SIZE (1024)
 
 struct pa_ioline {
     struct pa_iochannel *io;
+    struct pa_defer_event *defer_event;
+    struct pa_mainloop_api *mainloop;
+    int ref;
     int dead;
 
     char *wbuf;
@@ -50,7 +54,7 @@ struct pa_ioline {
 };
 
 static void io_callback(struct pa_iochannel*io, void *userdata);
-static int do_write(struct pa_ioline *l);
+static void defer_callback(struct pa_mainloop_api*m, struct pa_defer_event*e, void *userdata);
 
 struct pa_ioline* pa_ioline_new(struct pa_iochannel *io) {
     struct pa_ioline *l;
@@ -68,31 +72,63 @@ struct pa_ioline* pa_ioline_new(struct pa_iochannel *io) {
 
     l->callback = NULL;
     l->userdata = NULL;
+    l->ref = 1;
+
+    l->mainloop = pa_iochannel_get_mainloop_api(io);
+
+    l->defer_event = l->mainloop->defer_new(l->mainloop, defer_callback, l);
+    l->mainloop->defer_enable(l->defer_event, 0);
     
     pa_iochannel_set_callback(io, io_callback, l);
     
     return l;
 }
 
-void pa_ioline_free(struct pa_ioline *l) {
+static void ioline_free(struct pa_ioline *l) {
     assert(l);
+
+    if (l->io)
         pa_iochannel_free(l->io);
     pa_xfree(l->wbuf);
     pa_xfree(l->rbuf);
     pa_xfree(l);
 }
 
+void pa_ioline_unref(struct pa_ioline *l) {
+    assert(l && l->ref >= 1);
+
+    if ((--l->ref) <= 0)
+        ioline_free(l);
+}
+
+struct pa_ioline* pa_ioline_ref(struct pa_ioline *l) {
+    assert(l && l->ref >= 1);
+
+    l->ref++;
+    return l;
+}
+
+void pa_ioline_close(struct pa_ioline *l) {
+    assert(l && l->ref >= 1);
+
+    l->dead = 1;
+    if (l->io) {
+        pa_iochannel_free(l->io);
+        l->io = NULL;
+    }
+}
+
 void pa_ioline_puts(struct pa_ioline *l, const char *c) {
     size_t len;
-    assert(l && c);
+    assert(l && c && l->ref >= 1 && !l->dead);
+
+    pa_ioline_ref(l);
     
     len = strlen(c);
     if (len > BUFFER_LIMIT - l->wbuf_valid_length)
         len = BUFFER_LIMIT - l->wbuf_valid_length;
 
-    if (!len)
+    if (len) {
-        return;
-
         assert(l->wbuf_length >= l->wbuf_valid_length);
         
         /* In case the allocated buffer is too small, enlarge it. */
@@ -119,17 +155,29 @@ void pa_ioline_puts(struct pa_ioline *l, const char *c) {
         memcpy(l->wbuf + l->wbuf_index + l->wbuf_valid_length, c, len);
         l->wbuf_valid_length += len;
 
-    do_write(l);
+        l->mainloop->defer_enable(l->defer_event, 1);
+    }
+
+    pa_ioline_unref(l);
 }
 
 void pa_ioline_set_callback(struct pa_ioline*l, void (*callback)(struct pa_ioline*io, const char *s, void *userdata), void *userdata) {
-    assert(l);
+    assert(l && l->ref >= 1);
     l->callback = callback;
     l->userdata = userdata;
 }
 
+static void failure(struct pa_ioline *l) {
+    assert(l && l->ref >= 1 && !l->dead);
+
+    pa_ioline_close(l);
+
+    if (l->callback)
+        l->callback(l, NULL, l->userdata);
+}
+
 static void scan_for_lines(struct pa_ioline *l, size_t skip) {
-    assert(l && skip < l->rbuf_valid_length);
+    assert(l && l->ref >= 1 && skip < l->rbuf_valid_length);
 
     while (!l->dead && l->rbuf_valid_length > skip) {
         char *e, *p;
@@ -162,12 +210,11 @@ static void scan_for_lines(struct pa_ioline *l, size_t skip) {
 }
 
 static int do_read(struct pa_ioline *l) {
+    assert(l && l->ref >= 1);
+
+    while (!l->dead && pa_iochannel_is_readable(l->io)) {
         ssize_t r;
         size_t len;
-    assert(l);
-
-    if (!pa_iochannel_is_readable(l->io))
-        return 0;
 
         len = l->rbuf_length - l->rbuf_index - l->rbuf_valid_length;
         
@@ -200,13 +247,17 @@ static int do_read(struct pa_ioline *l) {
         assert(len >= READ_SIZE);
         
         /* Read some data */
-    if ((r = pa_iochannel_read(l->io, l->rbuf+l->rbuf_index+l->rbuf_valid_length, len)) <= 0)
+        if ((r = pa_iochannel_read(l->io, l->rbuf+l->rbuf_index+l->rbuf_valid_length, len)) <= 0) {
+            pa_log(__FILE__": read() failed: %s\n", r < 0 ? strerror(errno) : "EOF");
+            failure(l);
             return -1;
+        }
         
         l->rbuf_valid_length += r;
         
         /* Look if a line has been terminated in the newly read data */
         scan_for_lines(l, l->rbuf_valid_length - r);
+    }
         
     return 0;
 }
@@ -214,13 +265,15 @@ static int do_read(struct pa_ioline *l) {
 /* Try to flush the buffer */
 static int do_write(struct pa_ioline *l) {
     ssize_t r;
-    assert(l);
+    assert(l && l->ref >= 1);
 
-    if (!l->wbuf_valid_length || !pa_iochannel_is_writable(l->io))
+    while (!l->dead && pa_iochannel_is_writable(l->io) && l->wbuf_valid_length) {
-        return 0;
         
-    if ((r = pa_iochannel_write(l->io, l->wbuf+l->wbuf_index, l->wbuf_valid_length)) < 0)
+        if ((r = pa_iochannel_write(l->io, l->wbuf+l->wbuf_index, l->wbuf_valid_length)) < 0) {
+            pa_log(__FILE__": write() failed: %s\n", r < 0 ? strerror(errno) : "EOF");
+            failure(l);
             return -1;
+        }
         
         l->wbuf_index += r;
         l->wbuf_valid_length -= r;
@@ -228,20 +281,38 @@ static int do_write(struct pa_ioline *l) {
         /* A shortcut for the next time */
         if (l->wbuf_valid_length == 0)
             l->wbuf_index = 0;
+    }
         
     return 0;
 }
 
 /* Try to flush read/write data */
+static void do_work(struct pa_ioline *l) {
+    assert(l && l->ref >= 1);
+
+    pa_ioline_ref(l);
+
+    l->mainloop->defer_enable(l->defer_event, 0);
+    
+    if (!l->dead)
+        do_write(l);
+
+    if (!l->dead)
+        do_read(l);
+
+    pa_ioline_unref(l);
+}
+
 static void io_callback(struct pa_iochannel*io, void *userdata) {
     struct pa_ioline *l = userdata;
-    assert(io && l);
+    assert(io && l && l->ref >= 1);
 
-    if ((!l->dead && do_write(l) < 0) ||
+    do_work(l);
-        (!l->dead && do_read(l) < 0)) {
+}
 
-        l->dead = 1;
+static void defer_callback(struct pa_mainloop_api*m, struct pa_defer_event*e, void *userdata) {
-        if (l->callback)
+    struct pa_ioline *l = userdata;
-            l->callback(l, NULL, l->userdata);
+    assert(l && l->ref >= 1 && l->mainloop == m && l->defer_event == e);
-    }
+
+    do_work(l);
 }

polyp/ioline.h

@@ -27,7 +27,9 @@
 struct pa_ioline;
 
 struct pa_ioline* pa_ioline_new(struct pa_iochannel *io);
-void pa_ioline_free(struct pa_ioline *l);
+void pa_ioline_unref(struct pa_ioline *l);
+struct pa_ioline* pa_ioline_ref(struct pa_ioline *l);
+void pa_ioline_close(struct pa_ioline *l);
 
 void pa_ioline_puts(struct pa_ioline *s, const char *c);
 void pa_ioline_set_callback(struct pa_ioline*io, void (*callback)(struct pa_ioline*io, const char *s, void *userdata), void *userdata);

polyp/mainloop.c

@@ -116,8 +116,7 @@ static void mainloop_io_enable(struct pa_io_event *e, enum pa_io_event_flags eve
         e->pollfd->events =
             (events & PA_IO_EVENT_INPUT ? POLLIN : 0) |
             (events & PA_IO_EVENT_OUTPUT ? POLLOUT : 0) |
-            POLLHUP |
+            POLLERR | POLLHUP;
-            POLLERR;
 }
 
 static void mainloop_io_free(struct pa_io_event *e) {

polyp/protocol-esound.c

@@ -267,8 +267,10 @@ static int esd_proto_connect(struct connection *c, esd_proto_t request, const vo
         }
 
         c->authorized = 1;
-        if (c->auth_timeout_event)
+        if (c->auth_timeout_event) {
             c->protocol->core->mainloop->time_free(c->auth_timeout_event);
+            c->auth_timeout_event = NULL;
+        }
     }
     
     ekey = *(uint32_t*)((uint8_t*) data+ESD_KEY_LEN);
@@ -301,11 +303,13 @@ static int esd_proto_stream_play(struct connection *c, esd_proto_t request, cons
     ss.rate = rate;
     format_esd2native(format, &ss);
 
-    if (!pa_sample_spec_valid(&ss))
+    if (!pa_sample_spec_valid(&ss)) {
+        pa_log(__FILE__": invalid sample specification\n");
         return -1;
+    }
 
     if (!(sink = pa_namereg_get(c->protocol->core, c->protocol->sink_name, PA_NAMEREG_SINK, 1))) {
-        pa_log(__FILE__": No output sink\n");
+        pa_log(__FILE__": no such sink\n");
         return -1;
     }
     
@@ -314,18 +318,18 @@ static int esd_proto_stream_play(struct connection *c, esd_proto_t request, cons
 
     pa_client_set_name(c->client, name);
 
-    assert(!c->input_memblockq);
+    assert(!c->sink_input && !c->input_memblockq);
+
+    if (!(c->sink_input = pa_sink_input_new(sink, name, &ss, 0, -1))) {
+        pa_log(__FILE__": failed to create sink input.\n");
+        return -1;
+    }
 
     l = (size_t) (pa_bytes_per_second(&ss)*PLAYBACK_BUFFER_SECONDS); 
     c->input_memblockq = pa_memblockq_new(l, 0, pa_frame_size(&ss), l/2, l/PLAYBACK_BUFFER_FRAGMENTS, c->protocol->core->memblock_stat);
-    assert(c->input_memblockq);
     pa_iochannel_socket_set_rcvbuf(c->io, l/PLAYBACK_BUFFER_FRAGMENTS*2);
     c->playback.fragment_size = l/10;
 
-    assert(!c->sink_input);
-    c->sink_input = pa_sink_input_new(sink, name, &ss, 0, -1);
-    assert(c->sink_input);
-
     c->sink_input->owner = c->protocol->module;
     c->sink_input->client = c->client;
     c->sink_input->peek = sink_input_peek_cb;
@@ -355,40 +359,48 @@ static int esd_proto_stream_record(struct connection *c, esd_proto_t request, co
     ss.rate = rate;
     format_esd2native(format, &ss);
 
-    if (!pa_sample_spec_valid(&ss))
+    if (!pa_sample_spec_valid(&ss)) {
+        pa_log(__FILE__": invalid sample specification.\n");
         return -1;
+    }
 
     if (request == ESD_PROTO_STREAM_MON) {
         struct pa_sink* sink;
 
-        if (!(sink = pa_namereg_get(c->protocol->core, c->protocol->sink_name, PA_NAMEREG_SINK, 1)))
+        if (!(sink = pa_namereg_get(c->protocol->core, c->protocol->sink_name, PA_NAMEREG_SINK, 1))) {
+            pa_log(__FILE__": no such sink.\n");
             return -1;
+        }
 
-        if (!(source = sink->monitor_source))
+        if (!(source = sink->monitor_source)) {
+            pa_log(__FILE__": no such monitor source.\n");
             return -1;
+        }
     } else {
         assert(request == ESD_PROTO_STREAM_REC);
         
-        if (!(source = pa_namereg_get(c->protocol->core, c->protocol->source_name, PA_NAMEREG_SOURCE, 1)))
+        if (!(source = pa_namereg_get(c->protocol->core, c->protocol->source_name, PA_NAMEREG_SOURCE, 1))) {
+            pa_log(__FILE__": no such source.\n");
             return -1;
         }
+    }
     
     strncpy(name, (char*) data + sizeof(int)*2, sizeof(name));
     name[sizeof(name)-1] = 0;
 
     pa_client_set_name(c->client, name);
 
-    assert(!c->output_memblockq);
+    assert(!c->output_memblockq && !c->source_output);
+
+    if (!(c->source_output = pa_source_output_new(source, name, &ss, -1))) {
+        pa_log(__FILE__": failed to create source output\n");
+        return -1;
+    }
 
     l = (size_t) (pa_bytes_per_second(&ss)*RECORD_BUFFER_SECONDS); 
     c->output_memblockq = pa_memblockq_new(l, 0, pa_frame_size(&ss), 0, 0, c->protocol->core->memblock_stat);
-    assert(c->output_memblockq);
     pa_iochannel_socket_set_sndbuf(c->io, l/RECORD_BUFFER_FRAGMENTS*2);
     
-    assert(!c->source_output);
-    c->source_output = pa_source_output_new(source, name, &ss, -1);
-    assert(c->source_output);
-    
     c->source_output->owner = c->protocol->module;
     c->source_output->client = c->client;
     c->source_output->push = source_output_push_cb;
@@ -894,19 +906,22 @@ static void do_work(struct connection *c) {
     assert(c->protocol && c->protocol->core && c->protocol->core->mainloop && c->protocol->core->mainloop->defer_enable);
     c->protocol->core->mainloop->defer_enable(c->defer_event, 0);
 
-/*     pa_log("DOWORK\n");  */
+/*     pa_log("DOWORK %i\n", pa_iochannel_is_hungup(c->io));   */
 
-    if (c->dead || !c->io)
+    if (!c->dead && pa_iochannel_is_readable(c->io))
-        return;
-
-    if (pa_iochannel_is_readable(c->io))
         if (do_read(c) < 0)
             goto fail;
 
-    if (pa_iochannel_is_writable(c->io))
+    if (!c->dead && pa_iochannel_is_writable(c->io))
         if (do_write(c) < 0)
             goto fail;
 
+    /* In case the line was hungup, make sure to rerun this function
+       as soon as possible, until all data has been read. */
+
+    if (!c->dead && pa_iochannel_is_hungup(c->io))
+        c->protocol->core->mainloop->defer_enable(c->defer_event, 1);
+    
     return;
 
 fail:

polyp/protocol-simple.c

@@ -180,9 +180,6 @@ static void do_work(struct connection *c) {
     assert(c->protocol && c->protocol->core && c->protocol->core->mainloop && c->protocol->core->mainloop->defer_enable);
     c->protocol->core->mainloop->defer_enable(c->defer_event, 0);
 
-    if (pa_iochannel_is_hungup(c->io))
-        goto fail;
-    
     if (pa_iochannel_is_writable(c->io))
         if (do_write(c) < 0)
             goto fail;
@@ -191,6 +188,9 @@ static void do_work(struct connection *c) {
         if (do_read(c) < 0)
             goto fail;
 
+    if (pa_iochannel_is_hungup(c->io))
+        c->protocol->core->mainloop->defer_enable(c->defer_event, 1);
+
     return;
 
 fail:

polyp/pstream.c

@@ -103,24 +103,21 @@ static void do_read(struct pa_pstream *p);
 static void do_something(struct pa_pstream *p) {
     assert(p);
 
-    if (p->dead)
-        return;
-    
     p->mainloop->defer_enable(p->defer_event, 0);
 
     pa_pstream_ref(p);
     
-    if (!p->dead && pa_iochannel_is_hungup(p->io)) {
+    if (!p->dead && pa_iochannel_is_readable(p->io))
-        p->dead = 1;
+        do_read(p);
-        if (p->die_callback)
-            p->die_callback(p, p->die_callback_userdata);
-    }
 
     if (!p->dead && pa_iochannel_is_writable(p->io))
         do_write(p);
 
-    if (!p->dead && pa_iochannel_is_readable(p->io))
+    /* In case the line was hungup, make sure to rerun this function
-        do_read(p);
+       as soon as possible, until all data has been read. */
+    
+    if (!p->dead && pa_iochannel_is_hungup(p->io))
+        p->mainloop->defer_enable(p->defer_event, 1);
     
     pa_pstream_unref(p);
 }

polyp/socket-client.c

@@ -330,6 +330,7 @@ struct pa_socket_client* pa_socket_client_new_string(struct pa_mainloop_api *m,
         case KIND_TCP6: {
             uint16_t port = default_port;
             char *h;
+            int ret;
             struct addrinfo hints, *res;
 
             if (!(h = parse_address(p, &port)))
@@ -338,7 +339,10 @@ struct pa_socket_client* pa_socket_client_new_string(struct pa_mainloop_api *m,
             memset(&hints, 0, sizeof(hints));
             hints.ai_family = kind == KIND_TCP4 ? AF_INET : (kind == KIND_TCP6 ? AF_INET6 : AF_UNSPEC);
             
-            if (getaddrinfo(h, NULL, &hints, &res) < 0 || !res || !res->ai_addr)
+            ret = getaddrinfo(h, NULL, &hints, &res);
+            pa_xfree(h);
+
+            if (ret < 0 || !res || !res->ai_addr)
                 return NULL;
 
             if (res->ai_family == AF_INET) {