thread local storage wasn't freed properly

Have client:

Code:

---

/* dgramclnt.c:
 *
 * Example datagram client:
 */
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <time.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <pthread.h>

/*
 * This saves lines of code later:
 */
static void print_error(const char *file, const int line, const char *func, const char *lib_fn,
  const char *err_msg)
{
  fprintf(stderr, "Program fails in %s:%s:%d:\n  after calling of %s: %s\n",
    file, func, line, lib_fn, err_msg);
  exit(1);
}

#define wrap_print_error(fn, err_msg) print_error(__FILE__, __LINE__, __FUNCTION__ , fn, err_msg)
#define wrap_print_sys_error(fn) wrap_print_error(fn, strerror(errno))
#define wrap_print_net_error(fn, errc) wrap_print_error(fn, gai_strerror(errc))
static int run_receiver=1;
/*
 * Wait for a response:
 */
static void *reader(void *data)
{
  int adr_srvr_ret_size;
  int err;
  int fd_server_socket=*(int *)data;
  struct sockaddr_in adr_srvr_ret;    /* AF_INET */
  char dgram[512];            /* Receive buffer */
  //
  adr_srvr_ret_size = sizeof(adr_srvr_ret);
  err = pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
  if (err != 0)
  {
    errno=err;
    wrap_print_sys_error("pthread_setcancelstate(3)");
  }
  err = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
  if (err != 0)
  {
    errno=err;
    wrap_print_sys_error("pthread_setcanceltype(3)");
  }
  while(run_receiver)
  {
    err = recvfrom(fd_server_socket, /* Socket */ dgram, /* Receiving buffer */ sizeof(dgram), 0,
      /* Flags: no options */ (struct sockaddr *)&adr_srvr_ret, &adr_srvr_ret_size);
      /* Addr len, in & out */
    if ( err < 0 )
    {
      wrap_print_sys_error("recvfrom(2)");
      break;
    }
    dgram[err] = 0; /* null terminate */
    /*
    * Report Result:
    */
    printf("Result from %s port %u :\n\t'%s'\n", inet_ntoa(adr_srvr_ret.sin_addr),
      (unsigned)ntohs(adr_srvr_ret.sin_port), dgram);
  }
  pthread_exit(0);
}

int main(int argc,char **argv)
{
  int err, res;
  int x;
  char *srvr_addr = NULL;
  struct sockaddr_in adr_srvr;/* AF_INET */
  int len_inet;              /* length  */
  int fd_server_socket;      /* Socket */
  char dgram[512];            /* Send buffer */
  pthread_t thread_reader, *pthread_reader;
  /* Use a server address from the command line, if one has been provided.
  * Otherwise, this program will default to using the arbitrary address 127.0.0.23:
  */
  if ( argc >= 2 )
  {
    /* Addr on cmdline: */
    srvr_addr = argv[1];
  }
  else
  {
    /* Use default address: */
    srvr_addr = "127.0.0.23";
  }
  /*
  * Create a socket address, to use
  * to contact the server with:  <$nopage>
  */
  memset(&adr_srvr,0,sizeof adr_srvr);
  adr_srvr.sin_family = AF_INET;
  adr_srvr.sin_port = htons(9090);
  adr_srvr.sin_addr.s_addr = inet_addr(srvr_addr);
  //
  if ( adr_srvr.sin_addr.s_addr == INADDR_NONE )
  {
    wrap_print_sys_error("inet_addr");
    goto l_err;
  }
  len_inet = sizeof adr_srvr;
  //
  /*
  * Create a UDP socket to use:
  */
  fd_server_socket = socket(AF_INET,SOCK_DGRAM,0);
  if ( fd_server_socket == -1 )
  {
    wrap_print_sys_error("bad address.");
    goto l_err;
  }
  if ((errno=pthread_create(&thread_reader,NULL,&reader,&fd_server_socket)))
  {
    wrap_print_sys_error("pthread_create");
    pthread_reader=NULL;
    goto l_err;
  }
  else
    pthread_reader=&thread_reader;
  while(1)
  {
    /*
    * Prompt user for a date format string:
    */
    fputs("\nEnter format string: ",stdout);
    if ( !fgets(dgram,sizeof dgram,stdin) )
      break;                        /* EOF */
    err = strlen(dgram);
    if ( (err > 0) && (dgram[err-1] == '\n') )
      dgram[err-1] = 0;  /* Stomp out newline */
    /*
    * Send format string to server:
    */
    err = sendto(fd_server_socket,  /* Socket to send result */ dgram, /* The datagram result to snd */
          strlen(dgram), /* The datagram lngth */ 0, /* Flags: no options */
          (struct sockaddr *)&adr_srvr,/* addr */ len_inet);  /* Server address length */
    if ( err < 0 )
    {
      wrap_print_sys_error("sendto(2)");
      goto l_err;
    }
    /*
    * Test if we asked for a server shutdown:
    */
    if ( !strcasecmp(dgram,"QUIT") )
      break; /* Yes, we quit too */
  }
  /*
  * Close the socket and exit:
  */
  putchar('\n');
  res=0;
l_err:
  run_receiver=0;
  if (pthread_reader)
  {
    //if ((errno=pthread_join(thread_reader,NULL)))
    //  wrap_print_sys_error("pthread_join(3)");
    if ((errno=pthread_cancel(thread_reader)))
      wrap_print_sys_error("pthread_cancel(3)");
  }
  sleep(1);
  if (fd_server_socket)
    close(fd_server_socket);
  //pthread_exit(&res);
  return res;
}

---

As you can see i just divide reader to own thread.
And server:

Code:

---

/* dgramsrvr.c:
 *
 * Example datagram server:
 */
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <time.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>

/*
 * This function reports the error and
 * exits back to the shell:
 */
 #define arrsize(x) sizeof(x)/sizeof(x[0])
static inline void print_error(const char *file, const int line, const char *func, const char *lib_fn,
  const char *err_msg)
{
  fprintf(stderr, "Program fails in %s:%s:%d:\n  after calling of %s: %s\n",
    file, func, line, lib_fn, err_msg);
  exit(1);
}

#define wrap_print_error(fn, err_msg) print_error(__FILE__, __LINE__, __FUNCTION__ , fn, err_msg)
#define wrap_print_sys_error(fn) wrap_print_error(fn, strerror(errno))
#define wrap_print_net_error(fn, errc) wrap_print_error(fn, gai_strerror(errc))


int main(int argc,char **argv)
{
  int z, res=1;
  char *srvr_addr = NULL;
  struct sockaddr_in adr_inet;/* AF_INET */
  struct sockaddr_in adr_clnt;/* AF_INET */
  int len_inet;                /* length  */
  int fd_server_socket;                        /* Socket */
  char dgram[512];        /* Recv buffer */
  char dtfmt[512];  /* Date/Time Result */
  time_t td;    /* Current Time and Date */
  struct tm tm;      /* Date time values */
  /*
  * Use a server address from the command
  * line, if one has been provided.
  * Otherwise, this program will default
  * to using the arbitrary address
  * 127.0.0.23:
  */
  if ( argc >= 2 )
  {
    /* Addr on cmdline: */
    srvr_addr = argv[1];
  }
  else
  {
    /* Use default address: */
    srvr_addr = "127.0.0.23";
  }
  /*
  * Create a UDP socket to use:
  */
  fd_server_socket = socket(AF_INET,SOCK_DGRAM,0);
  if ( fd_server_socket == -1 )
  {
    wrap_print_sys_error("socket()");
    goto l_err;
  }
  /*
  * Create a socket address, for use
  * with bind(2):
  */
  memset(&adr_inet,0,sizeof adr_inet);
  adr_inet.sin_family = AF_INET;
  adr_inet.sin_port = htons(9090);
  adr_inet.sin_addr.s_addr = inet_addr(srvr_addr);
  if ( adr_inet.sin_addr.s_addr == INADDR_NONE )
  {
    wrap_print_sys_error("bad address.");
    goto l_err;
  }
  len_inet = sizeof(adr_inet);
  /*
  * Bind a address to our socket, so that
  * client programs can contact this
  * server:
  */
  z = bind(fd_server_socket, (struct sockaddr *)&adr_inet, len_inet);
  if ( z == -1 )
  {
    wrap_print_sys_error("bind()");
    goto l_err;
  }
  /*
  * Now wait for requests:
  */
  for (;;)
  {
    /*
    * Block until the program receives a datagram at our address and port:
    */
    len_inet = sizeof(adr_clnt);
    z = recvfrom(fd_server_socket,                /* Socket */
        dgram,          /* Receiving buffer */
        sizeof(dgram),  /* Max recv buf size */
        0,              /* Flags: no options */
        (struct sockaddr *)&adr_clnt,/* Addr */
        &len_inet);    /* Addr len, in & out */
    if ( z < 0 )
    {
      wrap_print_sys_error("recvfrom(2)");
      goto l_err;
    }
    printf("Client is host=%s port=%u;\n", inet_ntoa(adr_clnt.sin_addr),
      (unsigned)ntohs(adr_clnt.sin_port));
    /*
    * Process the request:
    */
    dgram[z] = 0; /* null terminate */
    if ( !strcasecmp(dgram,"QUIT") )
        break; /* Quit server */
    /*
    * Get the current date and time:
    */
    time(&td);    /* Get current time & date */
    tm = *localtime(&td);  /* Get components */
    /*
    * Format a new date and time string,
    * based upon the input format string:
    */
    {
      size_t date_res_lenght;
      //
      date_res_lenght=strftime(dtfmt, /* Formatted result */ sizeof dtfmt, /* Max result size */
        dgram, /* Input date/time format */ &tm); /* Input date/time values */
      /*
      * Send the formatted result back to the client program:
      */
      if (date_res_lenght==0)
      {
        static char *date_warn="Format not supported or lead to 0 length";
        //
        z = sendto(fd_server_socket,  /* Socket to send result */
          date_warn, /* The datagram result to snd */
          arrsize(date_warn)*sizeof(char), /* The datagram lngth */
          0,              /* Flags: no options */
          (struct sockaddr *)&adr_clnt,/* addr */
          len_inet);  /* Client address length */
      }
      else
        z = sendto(fd_server_socket,  /* Socket to send result */
          dtfmt, /* The datagram result to snd */
          strlen(dtfmt), /* The datagram lngth */
          0,              /* Flags: no options */
          (struct sockaddr *)&adr_clnt,/* addr */
          len_inet);  /* Client address length */
    }
    if ( z < 0 )
    {
      wrap_print_sys_error("sendto(2)");
      goto l_err;
    }
  }
  res=0;
l_err:
  /*
  * Close the socket and exit:
  */
  if (fd_server_socket)
    close(fd_server_socket);
  return res;
}

---

Everything work but valgrind shows that TLS memory isn't freed properly:

Quote:

---

valgrind --tool=memcheck --leak-check=full --track-origins=yes -v -v -v --show-reachable=yes ./udp_client

Quote:

---

272 bytes in 1 blocks are possibly lost in loss record 1 of 1
==11639== at 0x4C29E46: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11639== by 0x4012084: _dl_allocate_tls (dl-tls.c:297)
==11639== by 0x4E3AABC: pthread_create@@GLIBC_2.2.5 (allocatestack.c:571)
==11639== by 0x4010D1: main (udp_client.c:124)
==11639==
==11639== LEAK SUMMARY:
==11639== definitely lost: 0 bytes in 0 blocks
==11639== indirectly lost: 0 bytes in 0 blocks
==11639== possibly lost: 272 bytes in 1 blocks
==11639== still reachable: 0 bytes in 0 blocks
==11639== suppressed: 0 bytes in 0 blocks

---

---

So,
1)
2) Why pthread_cancel doesn't work properly despite PTHREAD_CANCEL_ENABLE/PTHREAD_CANCEL_ASYNCHRONOUS done?

main itself also needs to call pthread_exit()

Found: join should be done not instread but after cancellation.

Quote:

---

Originally Posted by Salem

main itself also needs to call pthread_exit()

---

Walking throught internet i really found that people use to do that, but it not seems to be necessary in my situation. And example from manual
PTHREAD_CANCEL(3)
calls
exit(EXIT_SUCCESS);
for main thread. Could you recall example when it really will be useful?

And yes, found another gap for me. Found that i can successfully change PTHREAD_CANCEL_ASYNCHRONOUS to PTHREAD_CANCEL_DEFERRED. I have doubt that i can do so despite POSIX requries recvfrom() to be cancellation point. But it only means that if i would ENTRY there then i MUST stop. And it doesnt mean that i WILL stop if i ALREADY inside. Or am i wrong?

>> while(run_receiver)
This is unsynchronized access to a shared variable, which is an error. Instead, you could loop until the socket is no longer valid. So the thread can "signaled" to exit by closing the socket.

>> if (pthread_join(run_receiver, NULL))
That should be "*pthread_reader" or "thread_reader". That may be your Valgrind leak, however, false-positives from Valgrind have been discussed before: http://cboard.cprogramming.com/c-pro...ad_create.html

>> arrsize(date_warn)
This is incorrect. date_warn is a pointer, not an array. To fix, declare date_warn as: static const char date_warn[] = "..." - or just use strlen() (which won't sent the null-terminator).

gg

The main reason for calling pthread_exit() from main() is:
1) all your other threads are detached or can be joined by some thread other than the main() thread
2) there is nothing for main() to do, and so the main() thread can end while allowing the other threads to complete
Because if you return or exit() from main(), then all threads are killed. But if you pthread_exit() from main(), then other threads in the process are allowed to complete.

recvfrom() is a cancellation point. So if you have a pending cancel, it should be acted upon when recvfrom() is called or while blocked inside it. However, I think pthread cancellation is overkill for this project.

gg

Quote:

---

Originally Posted by Codeplug

>> while(run_receiver)
This is unsynchronized access to a shared variable, which is an error. Instead, you could loop until the socket is no longer valid. So the thread can "signaled" to exit by closing the socket.

---

Yes, i thought about that, but it's OK, i don't see nothing wrong if loop runs one/two more time(s). It's rhather hack there.

Quote:

---

Originally Posted by Codeplug

>> if (pthread_join(run_receiver, NULL))
That should be "*pthread_reader" or "thread_reader". That may be your Valgrind leak, however, false-positives from Valgrind have been discussed before: http://cboard.cprogramming.com/c-pro...ad_create.html

---

Yes, i sow that, repaired that in 1st post, wonder even why it works before ))). And yes, they wasn't reason to valgrind fail (read above).
Anyway thanks twice coz you grabbed old version of code even before i repair it!

Quote:

---

Originally Posted by Codeplug

>> arrsize(date_warn)
This is incorrect. date_warn is a pointer, not an array. To fix, declare date_warn as: static const char date_warn[] = "..." - or just use strlen() (which won't sent the null-terminator).

---

Yep, my fail! Thank you.
Now actually have new questions (see above)

Quote:

---

Originally Posted by Codeplug

or while blocked inside it.

---

Clear. Thanks

Quote:

---

Originally Posted by Codeplug

However, I think pthread cancellation is overkill for this project.

---

Yep, other way i see is to do via signals. More actually lines of code with correct masking (only one thread for each signal, actually i should do that in original code, should i?) and supporting of chain of handlers.
Or is there smth even better?

>> Found: join should be done not instead [of] but after cancellation.
Correct. Think of pthread_create as a "malloc" and pthread_join (or detachment) as a "free" - they should always be paired.

>> Or is there smth even better?
Depends on what your real code is doing and what you're trying to achieve. If you are just looking to stop the thread, and the thread is doing nothing but recvfrom() calls, then closing the socket is the easiest way to "tell" the thread to exit.

gg

> Or is there smth even better?
Like single threaded code, and using select() with a timeout to monitor changes in the state of one (or more) file descriptors.

Single threaded code is so much easier to debug.

Unless this is of course just an exercise in using threads, in which case - carry on.