Question on thread

[flexo87]

Hi!, i'm having a problem with the deallocation of memory using threads. in the function that threads execute there are 3 data structures allocated with malloc(). So in the cleanup function i wanted to call the free() on them in order to avoid dangling pointers.
the structures are:

char ** response;
message_t *msg_r, *msg_s;

where message_t is:

typedef struct {
    char type;         
    unsigned int length;
    char* buffer;        //This is the pointer i want to be freed.
} message_t;

so what i do to pass these 3 pointers to the cleanup function is this:

void *serve_client(void *cl) {
      ..............
      void* p[3];
      p[0]=response;
      p[1]=msg_r->buffer;
      p[2]=msg_s->buffer;
      pthread_cleanup_push(cleanup, p);
      .........................
}

While this is the cleanup function:

static void cleanup(void *arg) {
	void **arr=(void**)arg;
	char **res=(char**)arr[0], *tmp,
		*buf1=(char*)arr[1],
		*buf2=(char*)arr[2];
	if (buf1) free(buf1), buf1=NULL;
	if (buf2) free(buf2), buf2=NULL;
	if (res) {
		tmp=*res;
		while (tmp)
			free(tmp++);
		free(res);
		res=NULL;
	}
	modify_many(0);
}

The problem is with the two buffers, cause if i don't put those two instructions (free(buf1), free(buf2)) i get this error:

*** glibc detected *** /homes/cls2/workspace/DPLAN/a.out: free(): invalid pointer: 0xb7fd6710 ***
======= Backtrace: =========
/lib/libc.so.6[0x1bc3a4]
/lib/libc.so.6(cfree+0x96)[0x1be356]
/homes/cls2/workspace/DPLAN/a.out[0x804ac48]
/homes/cls2/workspace/DPLAN/a.out[0x804adfa]
/lib/libpthread.so.0[0x13951f]
/lib/libc.so.6(clone+0x5e)[0x23104e]
======= Memory map: ========
00110000-00130000 r-xp 00000000 08:02 516844     /lib/ld-2.9.so
00130000-00131000 r-xp 00130000 00:00 0          [vdso]
00131000-00132000 r--p 00020000 08:02 516844     /lib/ld-2.9.so
00132000-00133000 rw-p 00021000 08:02 516844     /lib/ld-2.9.so
00133000-00149000 r-xp 00000000 08:02 516797     /lib/libpthread-2.9.so
00149000-0014a000 r--p 00015000 08:02 516797     /lib/libpthread-2.9.so
0014a000-0014b000 rw-p 00016000 08:02 516797     /lib/libpthread-2.9.so
0014b000-0014d000 rw-p 0014b000 00:00 0 
0014d000-002bb000 r-xp 00000000 08:02 516686     /lib/libc-2.9.so
002bb000-002bd000 r--p 0016e000 08:02 516686     /lib/libc-2.9.so
002bd000-002be000 rw-p 00170000 08:02 516686     /lib/libc-2.9.so
002be000-002c1000 rw-p 002be000 00:00 0 
002c1000-002ce000 r-xp 00000000 08:02 515090     /lib/libgcc_s-4.3.2-20081105.so.1
002ce000-002cf000 rw-p 0000c000 08:02 515090     /lib/libgcc_s-4.3.2-20081105.so.1
08048000-0804c000 r-xp 00000000 00:1f 9922589    /homes/cls2/workspace/DPLAN/a.out
0804c000-0804d000 rw-p 00004000 00:1f 9922589    /homes/cls2/workspace/DPLAN/a.out
0804d000-08071000 rw-p 0804d000 00:00 0          [heap]
b75d4000-b75d5000 ---p b75d4000 00:00 0 
b75d5000-b7fd7000 rw-p b75d5000 00:00 0 
b7fff000-b8000000 rw-p b7fff000 00:00 0 
bffeb000-c0000000 rw-p bffeb000 00:00 0          [stack]

Program received signal SIGABRT, Aborted.
[Switching to Thread 0xb7fd4b90 (LWP 18222)]
0x00130416 in __kernel_vsyscall ()
Missing separate debuginfos, use: debuginfo-install glibc-2.9-3.i686 libgcc-4.3.2-7.i386
(gdb) bt
#0  0x00130416 in __kernel_vsyscall ()
#1  0x00178460 in raise () from /lib/libc.so.6
#2  0x00179e28 in abort () from /lib/libc.so.6
#3  0x001b5fed in __libc_message () from /lib/libc.so.6
#4  0x001bc3a4 in malloc_printerr () from /lib/libc.so.6
#5  0x001be356 in free () from /lib/libc.so.6
#6  0x0804ac48 in cleanup (arg=0xb7fd4368) at dpserver.c:36
#7  0x0804adfa in serve_client (cl=0xbffff1ac) at dpserver.c:77
#8  0x0013951f in start_thread () from /lib/libpthread.so.0
#9  0x0023104e in clone () from /lib/libc.so.6
(gdb)

the msg_s (message to be sent) buffer is allocated in the thread function, while the msg_r (message to be received) buffer is allocated in a function that reads from a socket.
I used to get an error similar to this one with the double free() detection, but this time seems to be different!...Any help would be much appreciated!

[flexo87]

There is some news,... the free() which fails is the second one (free(buf2)), so is the pointer that i allocate in the thread function that is detected as an invalid pointer......why??

[Dino]

I don't understand what you are doing here:

static void cleanup(void *arg) {
	void **arr=(void**)arg;
	char **res=(char**)arr[0], *tmp,
		*buf1=(char*)arr[1],
		*buf2=(char*)arr[2];

Why the nested assignments as part of the declaration? Why the recast of arg? That's some butt-ugly code.

[flexo87]

cause i want to pass all the structures, that have some memory reserved, to the cleanup function that take a void * p as parameter and i'm giveing it a void **p, shouldn't i do it?

[vart]

void* p[3];

Are you sure that this local var is still available when you are calling your cleanup function?

[flexo87]

I think so...it should be! I don't call the cleanup function, i push it.

[Codeplug]

>> p[2]=msg_s->buffer;
p[2] now contains a copy of the value of buffer at that point in time.

>> the msg_s buffer is allocated in the thread function
Is that before or after you made the copy into p[2]? Keep in mind that you are taking a "snapshot" of pointer values. If they change for any reason (like due to a call to realloc), then the change won't be reflected in the snapshot.

gg

[flexo87]

the type of msg_s->buffer is char *, so p[2] that is a char * too, take the value of the pointer to the buffer, the malloc() is called after the copy.

[Codeplug]

>> the malloc() is called after the copy
There's your problem.

gg

[flexo87]

Wow...you're right cheers a lot!...I'm still surprised cause the first pointer (char** response) is allocated after too! But it's working fine! ...that will be one of the several undefined behavior of C !

Let me ask you a different question:
Do you think that if a Makefile is not written perfectly (i mean the dependencies), the program, even if compiles, could run and generate some error.
Actually my application open a directory and read all the files parsing the contents and generates syntax errors if the files don't respect the syntax.
Well, if i compile it "manually" (gcc f1.c f2.c....fn.c) it works, if i use tha Makefile, it compiles but gives me syntax errors, with the same directory!!!

[Codeplug]

There is an extra level of indirection with "response". The value of "response" never changes after the copy. What does change is "*response", "*(response + 1)", etc...

Pointers are just like any other variable:

int a = 1;
int b = a;
b = 2; // don't expect 'a' to change

int *pa = &a;
int *p = pa;
p = 0; // don't expect 'pa' to change

If you really want to be immutable to change then you could do something like this:

      p[0] = &response;
      p[1] = &msg_r;
      p[2] = &msg_s;

Now the variables themselves can change value and the cleanup function would still be able to get at what needs to be free'd.

>> if i use a Makefile, it compiles but gives me syntax errors
Think of 'make' as just a tool for invoking the the compiler and other tools. You can reproduce anything the Makefile does on the command line. With a different set of compiler options, you could end up with different warning/errors/behavior.

gg