sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2024-12-22 13:34:15 +03:00
Code

- merge volkers debug changes

Browse Source 
- 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)
This commit is contained in:
Andrew Tridgell 2007-04-18 11:20:24 +10:00
commit 8f059f4d91
14 changed files with 166 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ctdb/Makefile.in
View File

@ -22,7 +22,7 @@ EVENTS_OBJ = lib/events/events.o lib/events/events_standard.o
CTDB_COMMON_OBJ = common/ctdb.o common/ctdb_daemon.o common/ctdb_client.o common/ctdb_io.o common/util.o common/ctdb_util.o \
common/ctdb_call.o common/ctdb_ltdb.o common/ctdb_lockwait.o common/ctdb_message.o \
common/cmdline.o lib/util/idtree.o lib/util/db_wrap.o
common/cmdline.o lib/util/idtree.o lib/util/db_wrap.o lib/util/debug.o
CTDB_TCP_OBJ = tcp/tcp_connect.o tcp/tcp_io.o tcp/tcp_init.o

28
ctdb/common/ctdb.c
View File

@ -192,31 +192,39 @@ uint32_t ctdb_get_num_nodes(struct ctdb_context *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 +260,9 @@ void ctdb_recv_pkt(struct ctdb_context *ctdb, uint8_t *data, uint32_t length)
__location__, hdr->operation));
break;
}
talloc_free(hdr);
done:
talloc_free(tmp_ctx);
}
/*

32
ctdb/common/ctdb_call.c
View File

@ -105,6 +105,7 @@ static int ctdb_call_local(struct ctdb_db_context *ctdb_db, struct ctdb_call *ca
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 +253,7 @@ void ctdb_request_dmaster(struct ctdb_context *ctdb, struct ctdb_req_header *hdr
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 +301,12 @@ void ctdb_request_dmaster(struct ctdb_context *ctdb, struct ctdb_req_header *hdr
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 +324,7 @@ void ctdb_request_dmaster(struct ctdb_context *ctdb, struct ctdb_req_header *hdr
ctdb_queue_packet(ctdb, &r->hdr);
}
talloc_free(r);
talloc_free(tmp_ctx);
}
@ -414,7 +422,7 @@ void ctdb_request_call(struct ctdb_context *ctdb, struct ctdb_req_header *hdr)
called when a CTDB_REPLY_CALL packet comes in
This packet comes in response to a CTDB_REQ_CALL request packet. It
contains any reply data freom the call
contains any reply data from the call
*/
void ctdb_reply_call(struct ctdb_context *ctdb, struct ctdb_req_header *hdr)
{
@ -422,7 +430,10 @@ void ctdb_reply_call(struct ctdb_context *ctdb, struct ctdb_req_header *hdr)
struct ctdb_call_state *state;
state = idr_find(ctdb->idr, hdr->reqid);
if (state == NULL) return;
if (state == NULL) {
DEBUG(0, ("reqid %d not found\n", hdr->reqid));
return;
}
if (!talloc_get_type(state, struct ctdb_call_state)) {
DEBUG(0,("ctdb idr type error at %s\n", __location__));
@ -435,10 +446,6 @@ void ctdb_reply_call(struct ctdb_context *ctdb, struct ctdb_req_header *hdr)
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 +495,8 @@ void ctdb_reply_dmaster(struct ctdb_context *ctdb, struct ctdb_req_header *hdr)
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 +628,7 @@ struct ctdb_call_state *ctdb_call_local_send(struct ctdb_db_context *ctdb_db,
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 +717,10 @@ struct ctdb_call_state *ctdb_daemon_call_send(struct ctdb_db_context *ctdb_db,
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 *state;
state = ctdb_call_local_send(ctdb_db, call, &header, &data);
talloc_free(data.dptr);
return state;
}
talloc_free(data.dptr);
@ -724,7 +737,7 @@ struct ctdb_call_state *ctdb_daemon_call_send(struct ctdb_db_context *ctdb_db,
*/
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 +755,7 @@ int ctdb_daemon_call_recv(struct ctdb_call_state *state, struct ctdb_call *call)
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;
}

46
ctdb/common/ctdb_client.c
View File

@ -75,19 +75,20 @@ void ctdb_reply_fetch_lock(struct ctdb_context *ctdb, struct ctdb_req_header *hd
struct ctdb_fetch_lock_state *state;
state = idr_find(ctdb->idr, hdr->reqid);
if (state == NULL) return;
if (state == NULL) {
DEBUG(0, ("reqid %d not found at %s\n", hdr->reqid,
__location__));
return;
}
if (!talloc_get_type(state, struct ctdb_fetch_lock_state)) {
DEBUG(0, ("ctdb idr type error at %s\n", __location__));
DEBUG(0, ("ctdb idr type error at %s, it's a %s\n",
__location__, talloc_get_name(state)));
return;
}
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 +98,35 @@ void ctdb_reply_fetch_lock(struct ctdb_context *ctdb, struct ctdb_req_header *hd
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 +149,9 @@ static void ctdb_client_read_cb(uint8_t *data, size_t cnt, void *args)
default:
DEBUG(0,("bogus operation code:%d\n",hdr->operation));
}
done:
talloc_free(tmp_ctx);
}
/*
@ -172,6 +183,12 @@ static int ux_socket_connect(struct ctdb_context *ctdb)
}
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 +277,7 @@ struct ctdb_call_state *ctdb_call_send(struct ctdb_db_context *ctdb_db,
#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
@ -575,7 +593,7 @@ struct ctdb_record_handle *ctdb_fetch_lock(struct ctdb_db_context *ctdb_db, TALL
talloc_set_destructor(h, fetch_lock_destructor);
ret = ctdb_ltdb_fetch(ctdb_db, key, &h->header, ctdb_db, data);
ret = ctdb_ltdb_fetch(ctdb_db, key, &h->header, h, data);
if (ret != 0) {
talloc_free(h);
return NULL;

16
ctdb/common/ctdb_daemon.c
View File

@ -130,10 +130,10 @@ static struct ctdb_call_state *ctdb_daemon_fetch_lock_send(struct ctdb_db_contex
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,7 @@ static struct ctdb_call_state *ctdb_daemon_fetch_lock_send(struct ctdb_db_contex
state = ctdb_daemon_call_send_remote(ctdb_db, call, header);
state->fetch_private = rec;
talloc_steal(state, rec);
return state;
}
@ -187,6 +188,7 @@ static void daemon_fetch_lock_complete(struct ctdb_call_state *state)
DEBUG(0,(__location__ " Failed to queue packet from daemon to client\n"));
}
talloc_free(r);
talloc_free(state);
}
/*
@ -370,6 +372,14 @@ static void daemon_request_call_from_client(struct ctdb_client *client,
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 +416,7 @@ static void daemon_incoming_packet(struct ctdb_client *client, void *data, size_
}
done:
talloc_free(data);
talloc_free(tmp_ctx);
}

7
ctdb/common/ctdb_ltdb.c
View File

@ -287,13 +287,6 @@ int ctdb_ltdb_lock_fetch_requeue(struct ctdb_db_context *ctdb_db,
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;
}

2
ctdb/include/ctdb_private.h
View File

@ -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;

3
ctdb/include/includes.h
View File

@ -6,6 +6,7 @@
#include "idtree.h"
#include "ctdb.h"
#include "lib/util/dlinklist.h"
#include "lib/util/debug.h"
typedef bool BOOL;
@ -14,7 +15,7 @@ typedef bool BOOL;
extern int LogLevel;
#define DEBUG(lvl, x) if ((lvl) <= LogLevel) (printf x)
#define DEBUG(lvl, x) if ((lvl) <= LogLevel) (do_debug x)
#define _PUBLIC_

2
ctdb/lib/talloc/talloc.c
View File

@ -1028,7 +1028,7 @@ static void talloc_report_null(void)
/*
report on any memory hanging off the null context
*/
static void talloc_report_null_full(void)
void talloc_report_null_full(void)
{
if (talloc_total_size(null_context) != 0) {
talloc_report_full(null_context, stderr);

1
ctdb/lib/talloc/talloc.h
View File

@ -142,6 +142,7 @@ void talloc_report_depth_file(const void *ptr, int depth, int max_depth, FILE *f
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);

40
ctdb/lib/util/debug.c Normal file
View File

@ -0,0 +1,40 @@
/*
Unix SMB/CIFS implementation.
ctdb debug functions
Copyright (C) Volker Lendecke 2007
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
#include "includes.h"
#include "system/time.h"
#include <unistd.h>
void do_debug(const char *format, ...)
{
struct timeval tm;
va_list ap;
char *s = NULL;
va_start(ap, format);
vasprintf(&s, format, ap);
va_end(ap);
gettimeofday(&tm, NULL);
printf("%-8.8d.%-6.6d [%d]: %s", (int)tm.tv_sec, (int)tm.tv_usec,
(int)getpid(), s);
fflush(stdout);
free(s);
}

21
ctdb/lib/util/debug.h Normal file
View File

@ -0,0 +1,21 @@
/*
Unix SMB/CIFS implementation.
ctdb debug functions
Copyright (C) Volker Lendecke 2007
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
void do_debug(const char *format, ...);

6
ctdb/tests/ctdb_fetch.c
View File

@ -142,6 +142,10 @@ static void bench_fetch(struct ctdb_context *ctdb, struct event_context *ev)
printf("Event loop failed!\n");
break;
}
if (LogLevel > 9) {
talloc_report_null_full();
}
}
printf("Fetch: %.2f msgs/sec\n", msg_count/end_timer());
@ -193,6 +197,8 @@ int main(int argc, const char *argv[])
}
}
/* talloc_enable_leak_report_full(); */
/* setup the remaining options for the main program to use */
extra_argv = poptGetArgs(pc);
if (extra_argv) {

12
ctdb/tests/fetch.sh
View File

@ -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