- merge volkers debug changes

- fixed memory leaks in the 3 packet receive routines. The problem was
  that the ctdb_call logic would occasionally complete and free a
  incoming packet, which would then be freed again in the packet
  receive routine. The solution is to make the packet a child of a
  temporary context in the receive routine then free that temporary
  context. That allows other routines to keep or free the packet if
  they want to, while allowing us to safely free it (via a free of the
  temporary context) in the receive function

(This used to be ctdb commit 304aaaa7235febbe97ff9ecb43875b7265ac48cd)

diff --cc ctdb/common/ctdb.c
index a69cbdbee726a1da1f9eb97b54038828c17e0ee6,57ebd131da1495313ec6afdd1a605b215eead329..60e1e6b90b4d20cd75733b8b2e6724d8dcd8e3af
--- 1/ctdb/common/ctdb.c
--- 2/ctdb/common/ctdb.c
+++ b/ctdb/common/ctdb.c
@@@ -192,31 -192,32 +192,39 @@@ uint32_t ctdb_get_num_nodes(struct ctdb
  */
  void ctdb_recv_pkt(struct ctdb_context *ctdb, uint8_t *data, uint32_t length)
  {
--      struct ctdb_req_header *hdr;
++      struct ctdb_req_header *hdr = (struct ctdb_req_header *)data;
++      TALLOC_CTX *tmp_ctx;
++
++      /* place the packet as a child of the tmp_ctx. We then use
++         talloc_free() below to free it. If any of the calls want
++         to keep it, then they will steal it somewhere else, and the
++         talloc_free() will only free the tmp_ctx */
++      tmp_ctx = talloc_new(ctdb);
++      talloc_steal(tmp_ctx, hdr);
  
        if (length < sizeof(*hdr)) {
                ctdb_set_error(ctdb, "Bad packet length %d\n", length);
--              return;
++              goto done;
        }
--      hdr = (struct ctdb_req_header *)data;
        if (length != hdr->length) {
                ctdb_set_error(ctdb, "Bad header length %d expected %d\n", 
                               hdr->length, length);
--              return;
++              goto done;
        }
  
        if (hdr->ctdb_magic != CTDB_MAGIC) {
                ctdb_set_error(ctdb, "Non CTDB packet rejected\n");
--              return;
++              goto done;
        }
  
        if (hdr->ctdb_version != CTDB_VERSION) {
                ctdb_set_error(ctdb, "Bad CTDB version 0x%x rejected\n", hdr->ctdb_version);
--              return;
++              goto done;
        }
  
-       DEBUG(3,(__location__ " ctdb request of type %d length %d from node %d to %d\n",
-                hdr->operation, hdr->length, hdr->srcnode, hdr->destnode));
+       DEBUG(3,(__location__ " ctdb request %d of type %d length %d from "
+                "node %d to %d\n", hdr->reqid, hdr->operation, hdr->length,
+                hdr->srcnode, hdr->destnode));
  
        switch (hdr->operation) {
        case CTDB_REQ_CALL:
@@@ -252,7 -253,7 +260,9 @@@
                         __location__, hdr->operation));
                break;
        }
--      talloc_free(hdr);
++
++done:
++      talloc_free(tmp_ctx);
  }
  
  /*

diff --cc ctdb/common/ctdb_call.c
index 65a1a5659c9a3017880a2897fc6b210950b48d55,aa88829fa1c3320b5a068444e9b3db12c0b05737..ee8a824fc9b0e1f1bf453d89e5ecfc8ada976c6a
--- 1/ctdb/common/ctdb_call.c
--- 2/ctdb/common/ctdb_call.c
+++ b/ctdb/common/ctdb_call.c
@@@ -105,6 -105,6 +105,7 @@@ static int ctdb_call_local(struct ctdb_
        if (c->reply_data) {
                call->reply_data = *c->reply_data;
                talloc_steal(ctdb, call->reply_data.dptr);
++              talloc_set_name_const(call->reply_data.dptr, __location__);
        } else {
                call->reply_data.dptr = NULL;
                call->reply_data.dsize = 0;
@@@ -252,6 -252,6 +253,7 @@@ void ctdb_request_dmaster(struct ctdb_c
        struct ctdb_ltdb_header header;
        struct ctdb_db_context *ctdb_db;
        int ret, len;
++      TALLOC_CTX *tmp_ctx;
  
        key.dptr = c->data;
        key.dsize = c->keylen;
@@@ -299,6 -299,6 +301,12 @@@
        len = offsetof(struct ctdb_reply_dmaster, data) + data.dsize;
        r = ctdb->methods->allocate_pkt(ctdb, len);
        CTDB_NO_MEMORY_FATAL(ctdb, r);
++
++      /* put the packet on a temporary context, allowing us to safely free
++         it below even if ctdb_reply_dmaster() has freed it already */
++      tmp_ctx = talloc_new(ctdb);
++      talloc_steal(tmp_ctx, r);
++
        talloc_set_name_const(r, "reply_dmaster packet");
        r->hdr.length    = len;
        r->hdr.ctdb_magic = CTDB_MAGIC;
@@@ -316,7 -316,7 +324,7 @@@
                ctdb_queue_packet(ctdb, &r->hdr);
        }
  
--      talloc_free(r);
++      talloc_free(tmp_ctx);
  }
  
  
@@@ -433,12 -436,12 +444,8 @@@ void ctdb_reply_call(struct ctdb_contex
        state->call.reply_data.dsize = c->datalen;
        state->call.status = c->status;
  
 -      talloc_steal(c, state);
 -
 -      /* get an extra reference here - this prevents the free in ctdb_recv_pkt()
 -         from freeing the data */
 -      (void)talloc_reference(state, c);
 +      talloc_steal(state, c);
  
-       /* get an extra reference here - this prevents the free in ctdb_recv_pkt()
-          from freeing the data */
-       (void)talloc_reference(state, c);
- 
        state->state = CTDB_CALL_DONE;
        if (state->async.fn) {
                state->async.fn(state);
@@@ -488,6 -491,6 +495,8 @@@ void ctdb_reply_dmaster(struct ctdb_con
  
        ctdb_call_local(ctdb_db, &state->call, &state->header, &data, ctdb->vnn);
  
++      talloc_steal(state, state->call.reply_data.dptr);
++
        state->state = CTDB_CALL_DONE;
        if (state->async.fn) {
                state->async.fn(state);
@@@ -619,6 -622,6 +628,7 @@@ struct ctdb_call_state *ctdb_call_local
        state->ctdb_db = ctdb_db;
  
        ret = ctdb_call_local(ctdb_db, &state->call, header, data, ctdb->vnn);
++      talloc_steal(state, state->call.reply_data.dptr);
  
        event_add_timed(ctdb->ev, state, timeval_zero(), call_local_trigger, state);
  
@@@ -707,7 -710,10 +717,10 @@@ struct ctdb_call_state *ctdb_daemon_cal
        if (ret != 0) return NULL;
  
        if (header.dmaster == ctdb->vnn && !(ctdb->flags & CTDB_FLAG_SELF_CONNECT)) {
-               return ctdb_call_local_send(ctdb_db, call, &header, &data);
 -              struct ctdb_call_state *result;
 -              result = ctdb_call_local_send(ctdb_db, call, &header, &data);
++              struct ctdb_call_state *state;
++              state = ctdb_call_local_send(ctdb_db, call, &header, &data);
+               talloc_free(data.dptr);
 -              return result;
++              return state;
        }
  
        talloc_free(data.dptr);
@@@ -724,7 -730,7 +737,7 @@@
  */
  int ctdb_daemon_call_recv(struct ctdb_call_state *state, struct ctdb_call *call)
  {
--      struct ctdb_record_handle *rec;
++      struct ctdb_fetch_handle *rec;
  
        while (state->state < CTDB_CALL_DONE) {
                event_loop_once(state->node->ctdb->ev);
@@@ -742,6 -748,6 +755,7 @@@
                rec->header = state->header;
                rec->data->dptr = talloc_steal(rec, state->call.reply_data.dptr);
                rec->data->dsize = state->call.reply_data.dsize;
++              talloc_set_name_const(rec->data->dptr, __location__);
                talloc_free(state);
                return 0;
        }

diff --cc ctdb/common/ctdb_client.c
index 1b5c80c6f47d252ef56ddd8439c3dc7fdc63211a,d14647ba9059d1963134ddcb548e605cae9ff715..aca243eda12ec22be02f2cc718fb184476a9f790
--- 1/ctdb/common/ctdb_client.c
--- 2/ctdb/common/ctdb_client.c
+++ b/ctdb/common/ctdb_client.c
@@@ -84,10 -89,10 +89,6 @@@ void ctdb_reply_fetch_lock(struct ctdb_
  
        state->r = talloc_steal(state, r);
  
--      /* get an extra reference here - this prevents the free in ctdb_recv_pkt()
--         from freeing the data */
--      (void)talloc_reference(state, r);
--
        state->state = CTDB_FETCH_LOCK_DONE;
  }
  
@@@ -97,28 -102,28 +98,35 @@@
  static void ctdb_client_read_cb(uint8_t *data, size_t cnt, void *args)
  {
        struct ctdb_context *ctdb = talloc_get_type(args, struct ctdb_context);
--      struct ctdb_req_header *hdr;
++      struct ctdb_req_header *hdr = (struct ctdb_req_header *)data;
++      TALLOC_CTX *tmp_ctx;
++
++      /* place the packet as a child of a tmp_ctx. We then use
++         talloc_free() below to free it. If any of the calls want
++         to keep it, then they will steal it somewhere else, and the
++         talloc_free() will be a no-op */
++      tmp_ctx = talloc_new(ctdb);
++      talloc_steal(tmp_ctx, hdr);
  
        if (cnt < sizeof(*hdr)) {
                ctdb_set_error(ctdb, "Bad packet length %d in client\n", cnt);
--              exit(1);
--              return;
++              exit(1); /* XXX - temporary for debugging */
++              goto done;
        }
--      hdr = (struct ctdb_req_header *)data;
        if (cnt != hdr->length) {
                ctdb_set_error(ctdb, "Bad header length %d expected %d in client\n", 
                               hdr->length, cnt);
--              return;
++              goto done;
        }
  
        if (hdr->ctdb_magic != CTDB_MAGIC) {
                ctdb_set_error(ctdb, "Non CTDB packet rejected in client\n");
--              return;
++              goto done;
        }
  
        if (hdr->ctdb_version != CTDB_VERSION) {
                ctdb_set_error(ctdb, "Bad CTDB version 0x%x rejected in client\n", hdr->ctdb_version);
--              return;
++              goto done;
        }
  
        switch (hdr->operation) {
@@@ -141,6 -146,6 +149,9 @@@
        default:
                DEBUG(0,("bogus operation code:%d\n",hdr->operation));
        }
++
++done:
++      talloc_free(tmp_ctx);
  }
  
  /*
@@@ -172,6 -177,6 +183,12 @@@ static int ux_socket_connect(struct ctd
  }
  
  
++struct ctdb_record_handle {
++      struct ctdb_db_context *ctdb_db;
++      TDB_DATA key;
++      TDB_DATA *data;
++      struct ctdb_ltdb_header header;
++};
  
  /*
    make a recv call to the local ctdb daemon - called from client context
@@@ -260,6 -265,6 +277,7 @@@ struct ctdb_call_state *ctdb_call_send(
  #if 0
        if (header.dmaster == ctdb->vnn && !(ctdb->flags & CTDB_FLAG_SELF_CONNECT)) {
                state = ctdb_call_local_send(ctdb_db, call, &header, &data);
++              talloc_free(data.dptr);
                return state;
        }
  #endif

diff --cc ctdb/common/ctdb_daemon.c
index 94aba21ffb1ab864b5c6753a6c31e6ad0a542491,94aba21ffb1ab864b5c6753a6c31e6ad0a542491..03cc1777e612d2ce36bf18f99f685cb91b43d20c
--- 1/ctdb/common/ctdb_daemon.c
--- 2/ctdb/common/ctdb_daemon.c
+++ b/ctdb/common/ctdb_daemon.c
@@@ -130,10 -130,10 +130,10 @@@ static struct ctdb_call_state *ctdb_dae
                                                           TDB_DATA *data)
  {
        struct ctdb_call *call;
--      struct ctdb_record_handle *rec;
++      struct ctdb_fetch_handle *rec;
        struct ctdb_call_state *state;
  
--      rec = talloc(mem_ctx, struct ctdb_record_handle);
++      rec = talloc(mem_ctx, struct ctdb_fetch_handle);
        CTDB_NO_MEMORY_NULL(ctdb_db->ctdb, rec);
  
        
@@@ -150,6 -150,6 +150,7 @@@
  
        state = ctdb_daemon_call_send_remote(ctdb_db, call, header);
        state->fetch_private = rec;
++      talloc_steal(state, rec);
  
        return state;
  }
@@@ -187,6 -187,6 +188,7 @@@ static void daemon_fetch_lock_complete(
                DEBUG(0,(__location__ " Failed to queue packet from daemon to client\n"));
        }
        talloc_free(r);
++      talloc_free(state);
  }
  
  /*
@@@ -370,6 -370,6 +372,14 @@@ static void daemon_request_call_from_cl
  static void daemon_incoming_packet(struct ctdb_client *client, void *data, size_t nread)
  {
        struct ctdb_req_header *hdr = data;
++      TALLOC_CTX *tmp_ctx;
++
++      /* place the packet as a child of a tmp_ctx. We then use
++         talloc_free() below to free it. If any of the calls want
++         to keep it, then they will steal it somewhere else, and the
++         talloc_free() will be a no-op */
++      tmp_ctx = talloc_new(client);
++      talloc_steal(tmp_ctx, hdr);
  
        if (hdr->ctdb_magic != CTDB_MAGIC) {
                ctdb_set_error(client->ctdb, "Non CTDB packet rejected in daemon\n");
@@@ -406,7 -406,7 +416,7 @@@
        }
  
  done:
--      talloc_free(data);
++      talloc_free(tmp_ctx);
  }
  
  

diff --cc ctdb/common/ctdb_ltdb.c
index 724271da3bdddf7619f550bafb20fcee0616b422,724271da3bdddf7619f550bafb20fcee0616b422..823c296be0dd29a8e6f48cb76c47b83c1143eb4c
--- 1/ctdb/common/ctdb_ltdb.c
--- 2/ctdb/common/ctdb_ltdb.c
+++ b/ctdb/common/ctdb_ltdb.c
@@@ -287,13 -287,13 +287,6 @@@ int ctdb_ltdb_lock_fetch_requeue(struc
                return -1;
        }
  
--      /* we get an extra reference to the packet here, to 
--         stop it being freed in the top level packet handler */
--      if (talloc_reference(ctdb_db, hdr) == NULL) {
--              talloc_free(h);
--              return -1;
--      }
--
        /* now tell the caller than we will retry asynchronously */
        return -2;
  }

diff --cc ctdb/include/ctdb_private.h
index be0fbf85ae0a9acc4a269ff0894c349dd6d1bb62,be0fbf85ae0a9acc4a269ff0894c349dd6d1bb62..6736ee4ab7ca44535e3bf0bd933ceda61fa87ce4
--- 1/ctdb/include/ctdb_private.h
--- 2/ctdb/include/ctdb_private.h
+++ b/ctdb/include/ctdb_private.h
@@@ -195,7 -195,7 +195,7 @@@ struct ctdb_call_state 
  
  
  /* used for fetch_lock */
--struct ctdb_record_handle {
++struct ctdb_fetch_handle {
        struct ctdb_db_context *ctdb_db;
        TDB_DATA key;
        TDB_DATA *data;

diff --cc ctdb/lib/talloc/talloc.h
index 75c130a2756db0f9693e160be0a0be5e2cb3fa53,75c130a2756db0f9693e160be0a0be5e2cb3fa53..bb068019cce451d187a925a49c343dd84b5df7f0
--- 1/ctdb/lib/talloc/talloc.h
--- 2/ctdb/lib/talloc/talloc.h
+++ b/ctdb/lib/talloc/talloc.h
@@@ -142,6 -142,6 +142,7 @@@ void talloc_report_depth_file(const voi
  void talloc_report_full(const void *ptr, FILE *f);
  void talloc_report(const void *ptr, FILE *f);
  void talloc_enable_null_tracking(void);
++void talloc_report_null_full(void);
  void talloc_disable_null_tracking(void);
  void talloc_enable_leak_report(void);
  void talloc_enable_leak_report_full(void);

diff --cc ctdb/tests/ctdb_fetch.c
index 91fff3825dfc9c32215fd57afa88cc0c38164fd5,c5356edab32fb39d1ab5b25e92f3d338a8979479..e084b1bad8af9f0d292e011aed5b46fef5f8f3a9
--- 1/ctdb/tests/ctdb_fetch.c
--- 2/ctdb/tests/ctdb_fetch.c
+++ b/ctdb/tests/ctdb_fetch.c
@@@ -193,6 -197,8 +197,8 @@@ int main(int argc, const char *argv[]
                }
        }
  
 -      talloc_enable_leak_report_full();
++      /* talloc_enable_leak_report_full(); */
+ 
        /* setup the remaining options for the main program to use */
        extra_argv = poptGetArgs(pc);
        if (extra_argv) {

diff --cc ctdb/tests/fetch.sh
index 157934caa0a984d47ec9f4088805e558871034f5,157934caa0a984d47ec9f4088805e558871034f5..9020fdc230e718e1a48ecab34efc9d6ee1836360
--- 1/ctdb/tests/fetch.sh
--- 2/ctdb/tests/fetch.sh
+++ b/ctdb/tests/fetch.sh
@@@ -3,15 -3,15 +3,15 @@@
  killall -q ctdb_fetch
  
  echo "Trying 2 nodes"
--bin/ctdb_fetch --nlist tests/nodes.txt --listen 127.0.0.2:9001 $* &
--bin/ctdb_fetch --nlist tests/nodes.txt --listen 127.0.0.1:9001 $* 
++$VALGRIND bin/ctdb_fetch --nlist tests/nodes.txt --listen 127.0.0.2:9001 $* &
++$VALGRIND bin/ctdb_fetch --nlist tests/nodes.txt --listen 127.0.0.1:9001 $* 
  
  killall -q ctdb_fetch
  
  echo "Trying 4 nodes"
--bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.4:9001 $* &
--bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.3:9001 $* &
--bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.2:9001 $* &
--bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.1:9001 $* 
++$VALGRIND bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.4:9001 $* &
++$VALGRIND bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.3:9001 $* &
++$VALGRIND bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.2:9001 $* &
++$VALGRIND bin/ctdb_fetch --nlist tests/4nodes.txt --listen 127.0.0.1:9001 $* 
  
  killall -q ctdb_fetch