thread local storage wasn't freed properly

This is a discussion on thread local storage wasn't freed properly within the C Programming forums, part of the General Programming Boards category; Have client: Code: /* dgramclnt.c: * * Example datagram client: */ #include <stdio.h> #include <unistd.h> #include <stdlib.h> #include <errno.h> #include ...

  1. #1
    Registered User
    Join Date
    Dec 2010
    Posts
    51

    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:
    valgrind --tool=memcheck --leak-check=full --track-origins=yes -v -v -v --show-reachable=yes ./udp_client
    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?
    Last edited by icegood; 12-14-2012 at 06:43 AM. Reason: found errors in orig code

  2. #2
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,546
    main itself also needs to call pthread_exit()
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  3. #3
    Registered User
    Join Date
    Dec 2010
    Posts
    51
    Found: join should be done not instread but after cancellation.

  4. #4
    Registered User
    Join Date
    Dec 2010
    Posts
    51
    Quote Originally Posted by Salem View Post
    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?

  5. #5
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,669
    >> 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: Memory leak on pthread_create?

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

  6. #6
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,669
    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

  7. #7
    Registered User
    Join Date
    Dec 2010
    Posts
    51
    Quote Originally Posted by Codeplug View Post
    >> 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 View Post
    >> 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: Memory leak on pthread_create?
    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 View Post
    >> 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)

  8. #8
    Registered User
    Join Date
    Dec 2010
    Posts
    51
    Quote Originally Posted by Codeplug View Post
    or while blocked inside it.
    Clear. Thanks

    Quote Originally Posted by Codeplug View Post
    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?

  9. #9
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,669
    >> 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

  10. #10
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,546
    > 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.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 11
    Last Post: 09-07-2012, 04:35 AM
  2. boost::thread and local random variables
    By h3ro in forum C++ Programming
    Replies: 10
    Last Post: 01-05-2011, 08:05 AM
  3. Is getting object from Thread local storage expensive?
    By 6tr6tr in forum C++ Programming
    Replies: 2
    Last Post: 04-21-2008, 08:08 AM
  4. This would be funnier if it wasn't (likely) true
    By 7smurfs in forum A Brief History of Cprogramming.com
    Replies: 5
    Last Post: 06-16-2005, 06:11 PM
  5. Thread Local Storage
    By Hunter2 in forum Windows Programming
    Replies: 6
    Last Post: 07-06-2004, 04:00 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21