ver 1.0.2 - fix 2 corner-case bugs

  * libevent may report a deleted event inside same loop.
    Avoid socket reuse for one loop.
  * release_server() from disconnect_client() didnt look
    it the packet was actually sent.

diff --git a/Makefile b/Makefile
index 5e642f521f1f12f97190b407fba23520469707ca..6152f45412e32027aebb3b222f203809619a3974 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -14,6 +14,8 @@ DIRS = etc src debian
 # keep autoconf stuff separate
 -include config.mak
 
+CFLAGS += -DDBGVER="\"compiled by <$${USER}@`hostname`> at `date '+%Y-%m-%d %H:%M:%S'`\""
+
 # calculate full-path values
 OBJS = $(SRCS:.c=.o)
 hdrs = $(addprefix $(srcdir)/src/, $(HDRS))

diff --git a/NEWS b/NEWS
index 99c4fb00e3848ad1d86989afd6c480c8ce7f47f8..8ab22bc64e32d9aba047b19bb147746adad20464 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,11 @@
 
+2007-03-28  -  PgBouncer 1.0.2  -  "Supersonic Spoon"
+
+  * libevent may report a deleted event inside same loop.
+    Avoid socket reuse for one loop.
+  * release_server() from disconnect_client() didnt look
+    it the packet was actually sent.
+
 2007-03-15  -  PgBouncer 1.0.1  -  "Alien technology"
 
   = Fixes =

diff --git a/configure.ac b/configure.ac
index 24231bc8258e950f26bce1c84467c41a3e342096..f422fb75eb138b3967c8ff39066643d22614dfda 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
 dnl Process this file with autoconf to produce a configure script.
 
-AC_INIT(pgbouncer, 1.0.1)
+AC_INIT(pgbouncer, 1.0.2)
 AC_CONFIG_SRCDIR(src/bouncer.h)
 AC_CONFIG_HEADER(config.h)
 

diff --git a/debian/changelog b/debian/changelog
index fd32287c7b07f65320f72e1bb2e3442da71b171a..0eb83bf9978cee8af2e7338e646b1499b2adf0db 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+pgbouncer (1.0.2) unstable; urgency=low
+
+  * 2 more bugs.
+
+ -- Marko Kreen <marko.kreen@skype.net>  Wed, 28 Mar 2007 12:04:39 +0300
+
 pgbouncer (1.0.1) unstable; urgency=low
 
   * Couple hotfixes.

diff --git a/src/admin.c b/src/admin.c
index 2a8da8ad311d64e6dd9ef2ceec910ee6354d06f6..c3e1f16784a9063d5f9ccd46952d10cde898d66e 100644 (file)
--- a/src/admin.c
+++ b/src/admin.c
@@ -687,7 +687,7 @@ static bool admin_show_version(PgSocket *admin)
        bool res;
        SEND_generic(res, admin, 'N',
                "ssss", "SNOTICE", "C00000",
-               "MPgBouncer version " PACKAGE_VERSION, "");
+               "M" FULLVER, "");
        if (res)
                res = admin_ready(admin, "SHOW");
        return res;

diff --git a/src/bouncer.h b/src/bouncer.h
index 5d4fc852aaac64f0650c6e6f99c8fe48e14092b8..4230e4ad4c26bd3d7e8d3d389ec42ca859c71624 100644 (file)
--- a/src/bouncer.h
+++ b/src/bouncer.h
@@ -24,15 +24,23 @@
 
 #include <event.h>
 
+#ifdef DBGVER
+#define FULLVER   PACKAGE_NAME " version " PACKAGE_VERSION " (" DBGVER ")"
+#else
+#define FULLVER   PACKAGE_NAME " version " PACKAGE_VERSION
+#endif
+
 /* each state corresponts to a list */
 enum SocketState {
        CL_FREE,                /* free_client_list */
+       CL_JUSTFREE,            /* justfree_client_list */
        CL_LOGIN,               /* login_client_list */
        CL_WAITING,             /* pool->waiting_client_list */
        CL_ACTIVE,              /* pool->active_client_list */
        CL_CANCEL,              /* pool->cancel_req_list */
 
        SV_FREE,                /* free_server_list */
+       SV_JUSTFREE,            /* justfree_server_list */
        SV_LOGIN,               /* pool->new_server_list */
        SV_IDLE,                /* pool->idle_server_list */
        SV_ACTIVE,              /* pool->active_server_list */

diff --git a/src/client.c b/src/client.c
index 5abb916b4e4af1732514cd5b0e85495f266da3c9..b1ad1f604a57c5b2c059d628de8747bcb58904fe 100644 (file)
--- a/src/client.c
+++ b/src/client.c
@@ -341,7 +341,14 @@ bool client_proto(SBuf *sbuf, SBufEvent evtype, MBuf *pkt, void *arg)
        PgSocket *client = arg;
 
        Assert(!is_server_socket(client));
-       Assert(client->state != SV_FREE);
+       Assert(client->sbuf.sock);
+       Assert(client->state != CL_FREE);
+
+       if (client->state == CL_JUSTFREE) {
+               /* SBuf should catch the case */
+               slog_warning(client, "state=CL_JUSTFREE, should not happen");
+               return false;
+       }
 
        switch (evtype) {
        case SBUF_EV_CONNECT_OK:

diff --git a/src/janitor.c b/src/janitor.c
index 2840397f791950ce1eb56f3b084178f7e8543d28..0278a52ee66d2d9eff2b8269a9dbd96a2c9e1cf8 100644 (file)
--- a/src/janitor.c
+++ b/src/janitor.c
@@ -223,7 +223,7 @@ static int per_loop_suspend(PgPool *pool)
 /*
  * this function is called for each event loop.
  */
-void per_loop_object_maint(void)
+void per_loop_maint(void)
 {
        List *item;
        PgPool *pool;

diff --git a/src/janitor.h b/src/janitor.h
index e37c6e1cb9de0eb23fe0eca381833d0f68ee756f..39b47d3ff415ee9a5e0b711a906a3f92c5dc41fe 100644 (file)
--- a/src/janitor.h
+++ b/src/janitor.h
@@ -19,6 +19,6 @@
 void janitor_setup(void);
 void config_postprocess(void);
 void resume_all(void);
-void per_loop_object_maint(void);
+void per_loop_maint(void);
 bool suspend_socket(PgSocket *sk);
 

diff --git a/src/list.h b/src/list.h
index 8bc285485a055fd698ff4b1031bcf3dd7cea8cfb..e02bb1d9e563a714226e6e6a509ea459b6f06da7 100644 (file)
--- a/src/list.h
+++ b/src/list.h
@@ -112,6 +112,32 @@ static inline List *list_first(List *list)
        return list->next;
 }
 
+/* put all elems in one list in the start of another list */
+static inline void list_prepend_list(List *src, List *dst)
+{
+       if (list_empty(src))
+               return;
+       src->next->prev = dst;
+       src->prev->next = dst->next;
+       dst->next->prev = src->prev;
+       dst->next = src->next;
+
+       src->next = src->prev = src;
+}
+
+/* put all elems in one list in the end of another list */
+static inline void list_append_list(List *src, List *dst)
+{
+       if (list_empty(src))
+               return;
+       src->next->prev = dst->prev;
+       src->prev->next = dst;
+       dst->prev->next = src->next;
+       dst->prev = src->prev;
+
+       src->next = src->prev = src;
+}
+
 /* remove first elem from list and return with casting */
 #define list_pop_type(list, typ, field) \
        (list_empty(list) ? NULL \
@@ -151,6 +177,13 @@ struct StatList {
        const char *name;
 };
 
+static inline void statlist_inc_count(StatList *list, int val)
+{
+       list->cur_count += val;
+       if (list->cur_count > list->max_count)
+               list->max_count = list->cur_count;
+}
+
 #define STATLIST(var) StatList var = { {&var.head, &var.head}, 0, 0, #var }
 
 static inline void statlist_reset(StatList *list)
@@ -161,25 +194,19 @@ static inline void statlist_reset(StatList *list)
 static inline void statlist_prepend(List *item, StatList *list)
 {
        list_prepend(item, &list->head);
-       list->cur_count ++;
-       if (list->cur_count > list->max_count)
-               list->max_count = list->cur_count;
+       statlist_inc_count(list, 1);
 }
 
 static inline void statlist_append(List *item, StatList *list)
 {
        list_append(item, &list->head);
-       list->cur_count ++;
-       if (list->cur_count > list->max_count)
-               list->max_count = list->cur_count;
+       statlist_inc_count(list, 1);
 }
 
 static inline void statlist_put_before(List *item, StatList *list, List *pos)
 {
        list_append(item, pos);
-       list->cur_count++;
-       if (list->cur_count > list->max_count)
-               list->max_count = list->cur_count;
+       statlist_inc_count(list, 1);
 }
 
 static inline void statlist_remove(List *item, StatList *list)
@@ -227,6 +254,20 @@ static inline List *statlist_pop(StatList *list)
        return item;
 }
 
+static inline void statlist_prepend_list(StatList *src, StatList *dst)
+{
+       list_prepend_list(&src->head, &dst->head);
+       statlist_inc_count(dst, src->cur_count);
+       src->cur_count = 0;
+}
+
+static inline void statlist_append_list(StatList *src, StatList *dst)
+{
+       list_append_list(&src->head, &dst->head);
+       statlist_inc_count(dst, src->cur_count);
+       src->cur_count = 0;
+}
+
 static inline List *statlist_first(StatList *list)
 {
        return list_first(&list->head);

diff --git a/src/main.c b/src/main.c
index 9fdffc3e85629fbba872eaaff6a596cf892deb4d..4f48eaa070f4f3a424c11b17040a9ee101c8d622 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -400,7 +400,8 @@ static void main_loop_once(void)
 {
        reset_time_cache();
        event_loop(EVLOOP_ONCE);
-       per_loop_object_maint();
+       per_loop_maint();
+       reuse_just_freed_objects();
 }
 
 /* boot everything */
@@ -418,7 +419,7 @@ int main(int argc, char *argv[])
                        cf_verbose++;
                        break;
                case 'V':
-                       printf("%s version %s\n", PACKAGE_NAME, PACKAGE_VERSION);
+                       printf("%s\n", FULLVER);
                        return 0;
                case 'd':
                        cf_daemon = 1;

diff --git a/src/objects.c b/src/objects.c
index e5e857f4a25a01cae8cdba0a49931add9738c578..69b4a28f5b80928aa094aba9a6fd412872f84037 100644 (file)
--- a/src/objects.c
+++ b/src/objects.c
@@ -36,6 +36,14 @@ STATLIST(free_client_list);
 STATLIST(free_server_list);
 STATLIST(login_client_list);
 
+/*
+ * libevent may still report events when event_del()
+ * is called from somewhere else.  So hide just freed
+ * PgSockets for one loop.
+ */
+STATLIST(justfree_client_list);
+STATLIST(justfree_server_list);
+
 /* how many client sockets are allocated */
 static int absolute_client_count = 0;
 /* how many server sockets are allocated */
@@ -148,6 +156,9 @@ void change_client_state(PgSocket *client, SocketState newstate)
        case CL_FREE:
                statlist_remove(&client->head, &free_client_list);
                break;
+       case CL_JUSTFREE:
+               statlist_remove(&client->head, &justfree_client_list);
+               break;
        case CL_LOGIN:
                statlist_remove(&client->head, &login_client_list);
                break;
@@ -169,9 +180,11 @@ void change_client_state(PgSocket *client, SocketState newstate)
        /* put to new location */
        switch (client->state) {
        case CL_FREE:
-               /* use LIFO the keep cache warm */
                statlist_prepend(&client->head, &free_client_list);
                break;
+       case CL_JUSTFREE:
+               statlist_append(&client->head, &justfree_client_list);
+               break;
        case CL_LOGIN:
                statlist_append(&client->head, &login_client_list);
                break;
@@ -199,6 +212,9 @@ void change_server_state(PgSocket *server, SocketState newstate)
        case SV_FREE:
                statlist_remove(&server->head, &free_server_list);
                break;
+       case SV_JUSTFREE:
+               statlist_remove(&server->head, &justfree_server_list);
+               break;
        case SV_LOGIN:
                statlist_remove(&server->head, &pool->new_server_list);
                break;
@@ -223,14 +239,16 @@ void change_server_state(PgSocket *server, SocketState newstate)
        /* put to new location */
        switch (server->state) {
        case SV_FREE:
-               /* use LIFO the keep cache warm */
                statlist_prepend(&server->head, &free_server_list);
                break;
+       case SV_JUSTFREE:
+               statlist_append(&server->head, &justfree_server_list);
+               break;
        case SV_LOGIN:
                statlist_append(&server->head, &pool->new_server_list);
                break;
        case SV_USED:
-               /* again, LIFO */
+               /* use LIFO */
                statlist_prepend(&server->head, &pool->used_server_list);
                break;
        case SV_TESTED:
@@ -610,7 +628,7 @@ void disconnect_server(PgSocket *server, bool notify, const char *reason)
                sbuf_answer(&server->sbuf, pkt_term, sizeof(pkt_term));
        sbuf_close(&server->sbuf);
 
-       change_server_state(server, SV_FREE);
+       change_server_state(server, SV_JUSTFREE);
 }
 
 /* drop client connection */
@@ -622,7 +640,8 @@ void disconnect_client(PgSocket *client, bool notify, const char *reason)
        case CL_ACTIVE:
                if (client->link) {
                        PgSocket *server = client->link;
-                       if (server->ready) {
+                       /* ->ready may be set before all is sent */
+                       if (server->ready && sbuf_has_no_state(&server->sbuf)) {
                                release_server(server);
                        } else {
                                server->link = NULL;
@@ -649,7 +668,7 @@ void disconnect_client(PgSocket *client, bool notify, const char *reason)
 
        sbuf_close(&client->sbuf);
 
-       change_client_state(client, CL_FREE);
+       change_client_state(client, CL_JUSTFREE);
 }
 
 /* the pool needs new connection, if possible */
@@ -820,7 +839,7 @@ void forward_cancel_request(PgSocket *server)
 
        SEND_CancelRequest(res, server, req->cancel_key);
 
-       change_client_state(req, CL_FREE);
+       change_client_state(req, CL_JUSTFREE);
 }
 
 bool use_client_socket(int fd, PgAddr *addr,
@@ -940,4 +959,24 @@ void tag_database_dirty(PgDatabase *db)
        }
 }
 
+/* move objects from justfree_* to free_* lists */
+void reuse_just_freed_objects(void)
+{
+       List *tmp, *item;
+       PgSocket *sk;
+
+       /*
+        * Obviously, if state would be set to *_FREE,
+        * they could be moved in one go.
+        */
+       statlist_for_each_safe(item, &justfree_client_list, tmp) {
+               sk = container_of(item, PgSocket, head);
+               change_client_state(sk, CL_FREE);
+       }
+       statlist_for_each_safe(item, &justfree_server_list, tmp) {
+               sk = container_of(item, PgSocket, head);
+               change_server_state(sk, SV_FREE);
+       }
+}
+
 

diff --git a/src/objects.h b/src/objects.h
index 9d95ad547be02fbbab05936d180111fef4c51044..317cf47ea82a726ad5851a2ce492c9e874547ac2 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -61,3 +61,5 @@ void for_each_server(PgPool *pool, void (*func)(PgSocket *sk));
 
 void create_auth_cache(void);
 
+void reuse_just_freed_objects(void);
+

diff --git a/src/sbuf.c b/src/sbuf.c
index a3a636f7eb3e6cabc6a58384afb3703b2e8890a3..223005c1d5745b038607e718f0ce53bc953d6322 100644 (file)
--- a/src/sbuf.c
+++ b/src/sbuf.c
@@ -246,6 +246,10 @@ static void sbuf_send_cb(int sock, short flags, void *arg)
 {
        SBuf *sbuf = arg;
 
+       /* sbuf was closed before in this loop */
+       if (!sbuf->sock)
+               return;
+
        /* prepare normal situation for sbuf_recv_cb() */
        sbuf->wait_send = 0;
        sbuf_wait_for_data(sbuf);
@@ -272,6 +276,8 @@ static bool sbuf_send_pending(SBuf *sbuf)
        int res, avail;
        uint8 *pos;
 
+       Assert(sbuf->dst || !sbuf->send_remain);
+
 try_more:
        /* how much data is available for sending */
        avail = sbuf->recv_pos - sbuf->send_pos;
@@ -280,6 +286,11 @@ try_more:
        if (avail == 0)
                return true;
 
+       if (sbuf->dst->sock == 0) {
+               log_error("sbuf_send_pending: no dst sock?");
+               return false;
+       }
+
        /* actually send it */
        pos = sbuf->buf + sbuf->send_pos;
        res = safe_send(sbuf->dst->sock, pos, avail, 0);
@@ -421,6 +432,10 @@ static void sbuf_recv_cb(int sock, short flags, void *arg)
        int free, ok;
        SBuf *sbuf = arg;
 
+       /* sbuf was closed before in this loop */
+       if (!sbuf->sock)
+               return;
+
        /* reading should be disabled when waiting */
        Assert(sbuf->wait_send == 0);
 
@@ -442,16 +457,18 @@ try_more:
 
        /* now handle it */
        ok = sbuf_process_pending(sbuf);
+       if (!ok)
+               return;
 
        /* if the buffer is full, there can be more data available */
-       if (ok && sbuf->recv_pos == cf_sbuf_len)
+       if (sbuf->recv_pos == cf_sbuf_len)
                goto try_more;
 
        /* clean buffer */
        sbuf_try_resync(sbuf);
 
        /* notify proto that all is sent */
-       if (sbuf->send_pos == sbuf->recv_pos && sbuf->pkt_remain == 0)
+       if (sbuf_has_no_state(sbuf))
                sbuf_call_proto(sbuf, SBUF_EV_FLUSH);
 }
 

diff --git a/src/server.c b/src/server.c
index 9c6c28de7c9109cd6f8ef193c9bfdb73ebd8bb42..9f4ee346f6c1d2f0a84538ec68eca30c5a5da440 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -230,6 +230,12 @@ bool server_proto(SBuf *sbuf, SBufEvent evtype, MBuf *pkt, void *arg)
        Assert(is_server_socket(server));
        Assert(server->state != SV_FREE);
 
+       if (server->state == SV_JUSTFREE) {
+               /* SBuf should catch the case */
+               slog_warning(server, "state=SV_JUSTFREE, should not happen");
+               return false;
+       }
+
        switch (evtype) {
        case SBUF_EV_RECV_FAILED:
                disconnect_server(server, false, "server conn crashed?");