[PATCH 00/10] io cleanups and fixes
Felipe Contreras
This is an important reorganization of the I/O code in order to keep a tighter
grasp into the status of the connections: open, closed, connecting.
This is important since we want to be absolutely sure that connections are
really closed.
Felipe Contreras (10):
io/ssl: remove unused field
io/ssl: remove extra dispose code
io/ssl: remove unused code
io: move dispose() code to finalize()
Add new status field to 'node'
io: properly set 'open' status
io: properly set 'connecting' status
io: add checks for 'closed' status
io: improve 'not connected' errors
io/node: always close
cvr/pn_direct_conn.c | 2 +-
io/pn_cmd_server.c | 26 +++++++-------------------
io/pn_http_server.c | 34 ++++++++++++----------------------
io/pn_node.c | 44 ++++++++++++++++++++++++--------------------
io/pn_node_private.h | 9 +++++++--
io/pn_ssl_conn.c | 49 ++++++++++++-------------------------------------
6 files changed, 63 insertions(+), 101 deletions(-)
Felipe Contreras
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
io/pn_http_server.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/io/pn_http_server.c b/io/pn_http_server.c
index 9328fdb..36ecf2a 100644
--- a/io/pn_http_server.c
+++ b/io/pn_http_server.c
@@ -46,7 +46,6 @@
struct PnHttpServer
{
PnNode parent;
- gboolean dispose_has_run;
guint parser_state;
gboolean waiting_response;
--
1.6.6.1
Felipe Contreras
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
io/pn_ssl_conn.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/io/pn_ssl_conn.c b/io/pn_ssl_conn.c
index 06550aa..513f7e8 100644
--- a/io/pn_ssl_conn.c
+++ b/io/pn_ssl_conn.c
@@ -406,15 +406,6 @@ dispose (GObject *obj)
pn_log ("begin");
- if (!conn->dispose_has_run)
- {
- conn->dispose_has_run = TRUE;
-
- pn_node_close (conn);
-
- g_free (conn->name);
- }
-
G_OBJECT_CLASS (parent_class)->dispose (obj);
pn_log ("end");
--
1.6.6.1
Felipe Contreras
1 files changed, 0 insertions(+), 22 deletions(-)
diff --git a/io/pn_ssl_conn.c b/io/pn_ssl_conn.c
index 513f7e8..f12e238 100644
--- a/io/pn_ssl_conn.c
+++ b/io/pn_ssl_conn.c
@@ -400,38 +400,16 @@ read_impl (PnNode *conn,
/* GObject stuff. */
static void
-dispose (GObject *obj)
-{
- PnNode *conn = PN_NODE (obj);
-
- pn_log ("begin");
-
- G_OBJECT_CLASS (parent_class)->dispose (obj);
-
- pn_log ("end");
-}
-
-static void
-finalize (GObject *obj)
-{
- G_OBJECT_CLASS (parent_class)->finalize (obj);
-}
-
-static void
class_init (gpointer g_class,
gpointer class_data)
{
PnNodeClass *conn_class = PN_NODE_CLASS (g_class);
- GObjectClass *gobject_class = G_OBJECT_CLASS (g_class);
conn_class->connect = &connect_impl;
conn_class->close = &close_impl;
conn_class->write = &write_impl;
conn_class->read = &read_impl;
- gobject_class->dispose = dispose;
- gobject_class->finalize = finalize;
-
parent_class = g_type_class_peek_parent (g_class);
}
--
1.6.6.1
Felipe Contreras
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
io/pn_cmd_server.c | 21 ++-------------------
io/pn_http_server.c | 20 +-------------------
io/pn_node.c | 16 ++++++----------
io/pn_node_private.h | 1 -
4 files changed, 9 insertions(+), 49 deletions(-)
diff --git a/io/pn_cmd_server.c b/io/pn_cmd_server.c
index 4e7b562..d9d0628 100644
--- a/io/pn_cmd_server.c
+++ b/io/pn_cmd_server.c
@@ -275,26 +275,10 @@ read_impl (PnNode *conn)
/* GObject stuff. */
static void
-dispose (GObject *obj)
-{
- PnCmdServer *cmd_conn = PN_CMD_SERVER (obj);
-
- pn_log ("begin");
-
- if (cmd_conn->cmdproc)
- {
- msn_cmdproc_destroy (cmd_conn->cmdproc);
- cmd_conn->cmdproc = NULL;
- }
-
- G_OBJECT_CLASS (parent_class)->dispose (obj);
-
- pn_log ("end");
-}
-
-static void
finalize (GObject *obj)
{
+ PnCmdServer *cmd_conn = PN_CMD_SERVER (obj);
+ msn_cmdproc_destroy (cmd_conn->cmdproc);
G_OBJECT_CLASS (parent_class)->finalize (obj);
}
@@ -308,7 +292,6 @@ class_init (gpointer g_class,
conn_class->parse = &parse_impl;
conn_class->close = &close_impl;
- gobject_class->dispose = dispose;
gobject_class->finalize = finalize;
parent_class = g_type_class_peek_parent (g_class);
diff --git a/io/pn_http_server.c b/io/pn_http_server.c
index 36ecf2a..b3a125f 100644
--- a/io/pn_http_server.c
+++ b/io/pn_http_server.c
@@ -983,32 +983,15 @@ write_impl (PnNode *conn,
/* GObject stuff. */
static void
-dispose (GObject *obj)
+finalize (GObject *obj)
{
PnHttpServer *http_conn = PN_HTTP_SERVER (obj);
- pn_log ("begin");
-
g_free (http_conn->old_buffer);
- http_conn->old_buffer = NULL;
-
g_free (http_conn->gateway);
- http_conn->gateway = NULL;
-
g_queue_free (http_conn->write_queue);
- http_conn->write_queue = NULL;
-
g_hash_table_destroy (http_conn->childs);
- http_conn->childs = NULL;
-
- G_OBJECT_CLASS (parent_class)->dispose (obj);
- pn_log ("end");
-}
-
-static void
-finalize (GObject *obj)
-{
G_OBJECT_CLASS (parent_class)->finalize (obj);
}
@@ -1024,7 +1007,6 @@ class_init (gpointer g_class,
conn_class->write = &write_impl;
conn_class->read = &read_impl;
- gobject_class->dispose = dispose;
gobject_class->finalize = finalize;
parent_class = g_type_class_peek_parent (g_class);
diff --git a/io/pn_node.c b/io/pn_node.c
index 5efbc8e..b4b1d4a 100644
--- a/io/pn_node.c
+++ b/io/pn_node.c
@@ -636,16 +636,6 @@ dispose (GObject *obj)
conn->next = NULL;
}
- if (!conn->dispose_has_run)
- {
- conn->dispose_has_run = TRUE;
-
- if (conn->stream)
- pn_node_close (conn);
-
- g_free (conn->name);
- }
-
parent_class->dispose (obj);
pn_log ("end");
@@ -654,6 +644,12 @@ dispose (GObject *obj)
static void
finalize (GObject *obj)
{
+ PnNode *conn = PN_NODE (obj);
+
+ if (conn->stream)
+ pn_node_close (conn);
+
+ g_free (conn->name);
parent_class->finalize (obj);
}
diff --git a/io/pn_node_private.h b/io/pn_node_private.h
index 48d508c..d959162 100644
--- a/io/pn_node_private.h
+++ b/io/pn_node_private.h
@@ -44,7 +44,6 @@ struct PnNode
{
GObject parent;
gboolean open;
- gboolean dispose_has_run;
GError *error; /**< The current IO error .*/
guint read_watch; /** < The source id of the read watch. */
--
1.6.6.1
Felipe Contreras
future.
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
cvr/pn_direct_conn.c | 2 +-
io/pn_http_server.c | 4 ++--
io/pn_node.c | 8 +++++---
io/pn_node_private.h | 8 +++++++-
io/pn_ssl_conn.c | 4 +++-
5 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/cvr/pn_direct_conn.c b/cvr/pn_direct_conn.c
index ef6e3d9..1e9e46a 100644
--- a/cvr/pn_direct_conn.c
+++ b/cvr/pn_direct_conn.c
@@ -202,7 +202,7 @@ open_cb(PnNode *conn,
g_signal_handler_disconnect(conn, direct_conn->open_handler);
direct_conn->open_handler = 0;
- if (!conn->open) {
+ if (!conn->status == PN_NODE_STATUS_OPEN) {
if (g_queue_is_empty(direct_conn->addrs)) {
pn_warning("no more addresses to try");
pn_direct_conn_destroy(direct_conn);
diff --git a/io/pn_http_server.c b/io/pn_http_server.c
index b3a125f..009ed3f 100644
--- a/io/pn_http_server.c
+++ b/io/pn_http_server.c
@@ -168,7 +168,7 @@ read_cb (GIOChannel *source,
}
}
- if (conn->open)
+ if (conn->status == PN_NODE_STATUS_OPEN)
{
http_conn->waiting_response = FALSE;
@@ -426,7 +426,7 @@ connect_cb (gpointer data,
g_io_channel_set_encoding (channel, NULL, NULL);
g_io_channel_set_line_term (channel, "\r\n", 2);
- conn->open = TRUE;
+ conn->status = PN_NODE_STATUS_OPEN;
http_conn->timer = pn_timer_new (http_poll, http_conn);
pn_timer_start (http_conn->timer, 2);
diff --git a/io/pn_node.c b/io/pn_node.c
index b4b1d4a..1d2e4ea 100644
--- a/io/pn_node.c
+++ b/io/pn_node.c
@@ -114,7 +114,7 @@ open_cb (PnNode *next,
pn_log ("begin");
- conn->open = TRUE;
+ conn->status = PN_NODE_STATUS_OPEN;
{
PnNodeClass *class;
@@ -362,7 +362,7 @@ connect_cb (gpointer data,
PN_NODE_GET_CLASS (conn)->channel_setup (conn, channel);
- conn->open = TRUE;
+ conn->status = PN_NODE_STATUS_OPEN;
pn_info ("connected: conn=%p,channel=%p", conn, channel);
conn->read_watch = g_io_add_watch (channel, G_IO_IN, read_cb, conn);
@@ -443,7 +443,7 @@ close_impl (PnNode *conn)
pn_info ("closing '%s'", conn->name);
pn_debug ("conn=%p,name=%s", conn, conn->name);
- conn->open = FALSE;
+ conn->status = PN_NODE_STATUS_CLOSED;
g_free (conn->hostname);
conn->hostname = NULL;
@@ -481,6 +481,8 @@ close_impl (PnNode *conn)
}
leave:
+ conn->status = PN_NODE_STATUS_CLOSED;
+
pn_log ("end");
}
diff --git a/io/pn_node_private.h b/io/pn_node_private.h
index d959162..39427fb 100644
--- a/io/pn_node_private.h
+++ b/io/pn_node_private.h
@@ -40,10 +40,16 @@ struct MsnSesion;
GQuark pn_node_error_quark (void);
+enum pn_node_status {
+ PN_NODE_STATUS_CLOSED,
+ PN_NODE_STATUS_CONNECTING,
+ PN_NODE_STATUS_OPEN,
+};
+
struct PnNode
{
GObject parent;
- gboolean open;
+ enum pn_node_status status;
GError *error; /**< The current IO error .*/
guint read_watch; /** < The source id of the read watch. */
diff --git a/io/pn_ssl_conn.c b/io/pn_ssl_conn.c
index f12e238..f20431a 100644
--- a/io/pn_ssl_conn.c
+++ b/io/pn_ssl_conn.c
@@ -166,7 +166,7 @@ connect_cb (gpointer data,
if (gsc->fd >= 0)
{
- conn->open = TRUE;
+ conn->status = PN_NODE_STATUS_OPEN;
pn_info ("connected: conn=%p", conn);
purple_ssl_input_add (gsc, read_cb, conn);
@@ -269,6 +269,8 @@ close_impl (PnNode *conn)
}
#endif /* HAVE_LIBPURPLE */
+ conn->status = PN_NODE_STATUS_CLOSED;
+
Felipe Contreras
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
io/pn_http_server.c | 2 ++
io/pn_node.c | 2 ++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/io/pn_http_server.c b/io/pn_http_server.c
index 009ed3f..0cebb8c 100644
--- a/io/pn_http_server.c
+++ b/io/pn_http_server.c
@@ -376,6 +376,8 @@ connect_cb(GObject *source,
g_io_channel_set_encoding (channel, NULL, NULL);
g_io_channel_set_line_term (channel, "\r\n", 2);
+ conn->status = PN_NODE_STATUS_OPEN;
+
http_conn->timer = pn_timer_new (http_poll, http_conn);
pn_timer_start (http_conn->timer, 2);
diff --git a/io/pn_node.c b/io/pn_node.c
index 1d2e4ea..2057983 100644
--- a/io/pn_node.c
+++ b/io/pn_node.c
@@ -317,6 +317,8 @@ connect_cb(GObject *source,
PN_NODE_GET_CLASS (conn)->channel_setup (conn, channel);
+ conn->status = PN_NODE_STATUS_OPEN;
+
pn_info("connected: conn=%p,channel=%p", conn, channel);
conn->read_watch = g_io_add_watch(channel, G_IO_IN, read_cb, conn);
#if 0
--
1.6.6.1
Felipe Contreras
io/pn_ssl_conn.c | 2 ++
3 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/io/pn_http_server.c b/io/pn_http_server.c
index 0cebb8c..65d9d72 100644
--- a/io/pn_http_server.c
+++ b/io/pn_http_server.c
@@ -465,6 +465,8 @@ connect_impl (PnNode *conn,
http_conn = PN_HTTP_SERVER (conn);
+ conn->status = PN_NODE_STATUS_CONNECTING;
+
if (!conn->stream)
{
port = 80;
diff --git a/io/pn_node.c b/io/pn_node.c
index 2057983..ab00430 100644
--- a/io/pn_node.c
+++ b/io/pn_node.c
@@ -412,6 +412,8 @@ connect_impl (PnNode *conn,
if (conn->next)
{
+ conn->status = PN_NODE_STATUS_CONNECTING;
+
conn->next->prev = conn;
pn_node_connect (conn->next, hostname, port);
conn->next->prev = NULL;
@@ -421,6 +423,8 @@ connect_impl (PnNode *conn,
if (conn->stream)
pn_node_close (conn);
+ conn->status = PN_NODE_STATUS_CONNECTING;
+
#if defined(USE_GIO)
GSocketClient *client;
client = g_socket_client_new();
diff --git a/io/pn_ssl_conn.c b/io/pn_ssl_conn.c
index f20431a..7858627 100644
--- a/io/pn_ssl_conn.c
+++ b/io/pn_ssl_conn.c
@@ -231,6 +231,8 @@ connect_impl (PnNode *conn,
pn_node_close (conn);
+ conn->status = PN_NODE_STATUS_CONNECTING;
+
#ifdef HAVE_LIBPURPLE
ssl_conn->ssl_data = purple_ssl_connect (msn_session_get_user_data (conn->session),
hostname, port, connect_cb, error_cb, ssl_conn);
--
1.6.6.1
Felipe Contreras
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
io/pn_cmd_server.c | 5 +++++
io/pn_http_server.c | 5 +++++
io/pn_node.c | 5 +++++
io/pn_ssl_conn.c | 5 +++++
4 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/io/pn_cmd_server.c b/io/pn_cmd_server.c
index d9d0628..d6196df 100644
--- a/io/pn_cmd_server.c
+++ b/io/pn_cmd_server.c
@@ -166,6 +166,11 @@ close_impl (PnNode *conn)
{
PnCmdServer *cmd_conn;
+ if (conn->status == PN_NODE_STATUS_CLOSED) {
+ pn_log ("already closed: %p", conn);
+ return;
+ }
+
pn_log ("begin");
cmd_conn = PN_CMD_SERVER (conn);
diff --git a/io/pn_http_server.c b/io/pn_http_server.c
index 65d9d72..d7d8d4d 100644
--- a/io/pn_http_server.c
+++ b/io/pn_http_server.c
@@ -507,6 +507,11 @@ close_impl (PnNode *conn)
{
PnHttpServer *http_conn;
+ if (conn->status == PN_NODE_STATUS_CLOSED) {
+ pn_log ("already closed: %p", conn);
+ return;
+ }
+
pn_log ("begin");
http_conn = PN_HTTP_SERVER (conn);
diff --git a/io/pn_node.c b/io/pn_node.c
index ab00430..42411df 100644
--- a/io/pn_node.c
+++ b/io/pn_node.c
@@ -444,6 +444,11 @@ close_impl (PnNode *conn)
{
g_return_if_fail (conn);
+ if (conn->status == PN_NODE_STATUS_CLOSED) {
+ pn_log ("already closed: %p", conn);
+ return;
+ }
+
pn_log ("begin");
pn_info ("closing '%s'", conn->name);
diff --git a/io/pn_ssl_conn.c b/io/pn_ssl_conn.c
index 7858627..b90f365 100644
--- a/io/pn_ssl_conn.c
+++ b/io/pn_ssl_conn.c
@@ -248,6 +248,11 @@ close_impl (PnNode *conn)
g_return_if_fail (conn);
+ if (conn->status == PN_NODE_STATUS_CLOSED) {
+ pn_log ("already closed: %p", conn);
+ return;
+ }
+
pn_log ("begin");
ssl_conn = PN_SSL_CONN (conn);
--
1.6.6.1
Felipe Contreras
to catch them.
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
io/pn_node.c | 7 ++-----
io/pn_ssl_conn.c | 7 ++-----
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/io/pn_node.c b/io/pn_node.c
index 42411df..8db488a 100644
--- a/io/pn_node.c
+++ b/io/pn_node.c
@@ -464,11 +464,6 @@ close_impl (PnNode *conn)
goto leave;
}
- if (!conn->stream)
- {
- pn_error ("not connected: conn=%p", conn);
- }
-
#if defined(USE_GIO)
g_object_unref(conn->socket_conn);
#elif defined(HAVE_LIBPURPLE)
@@ -490,6 +485,8 @@ close_impl (PnNode *conn)
pn_stream_free (conn->stream);
conn->stream = NULL;
}
+ else
+ pn_error ("not connected: conn=%p", conn);
leave:
conn->status = PN_NODE_STATUS_CLOSED;
diff --git a/io/pn_ssl_conn.c b/io/pn_ssl_conn.c
index b90f365..6c28480 100644
--- a/io/pn_ssl_conn.c
+++ b/io/pn_ssl_conn.c
@@ -262,11 +262,6 @@ close_impl (PnNode *conn)
g_free (conn->hostname);
conn->hostname = NULL;
- if (!ssl_conn->ssl_data)
- {
- pn_warning ("not connected: conn=%p", conn);
- }
-
#ifdef HAVE_LIBPURPLE
if (ssl_conn->ssl_data)
{
@@ -274,6 +269,8 @@ close_impl (PnNode *conn)
purple_ssl_close (ssl_conn->ssl_data);
ssl_conn->ssl_data = NULL;
}
+ else
+ pn_error ("not connected: conn=%p", conn);
#endif /* HAVE_LIBPURPLE */
conn->status = PN_NODE_STATUS_CLOSED;
--
1.6.6.1
Felipe Contreras
Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
io/pn_node.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/io/pn_node.c b/io/pn_node.c
index 8db488a..278f2ed 100644
--- a/io/pn_node.c
+++ b/io/pn_node.c
@@ -420,8 +420,7 @@ connect_impl (PnNode *conn,
}
else
{
- if (conn->stream)
- pn_node_close (conn);
+ pn_node_close (conn);
conn->status = PN_NODE_STATUS_CONNECTING;
@@ -656,8 +655,7 @@ finalize (GObject *obj)
{
PnNode *conn = PN_NODE (obj);
- if (conn->stream)
- pn_node_close (conn);
+ pn_node_close (conn);
g_free (conn->name);
parent_class->finalize (obj);
--
1.6.6.1