ActiveMQ C++ Client
  1. ActiveMQ C++ Client
  2. AMQCPP-529

Crash in Threads due to small stack size (set to 32768 bytes)

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 3.8.1
    • Fix Version/s: 3.8.3, 3.9.0
    • Component/s: Decaf
    • Labels:
      None
    • Environment:

      linux50

      Description

      Using version 3.8.1 of activemq-cpp (after using version 3.4.5) - we have suffered a crash in code running in amq threads.
      Investigation shows that this happens when trying to use a char[] buffer on stack of size > 16384 , probably due to the stack size being of size 32768.

      Thread::Thread() constructors call initializeSelf() with stackSize=(-1), which
      in turn call createThreadInstance() with negative stackSize value (hence using PLATFORM_DEFAULT_STACK_SIZE).

      Relevant code below:
      ----------------------
      #define PLATFORM_DEFAULT_STACK_SIZE 0x8000
      -----------------------
      void createThreadInstance(ThreadHandle* thread, long long stackSize, int priority, bool suspended, threadingTask threadMain, void* threadArg) {
      if (stackSize <= 0)

      { stackSize = PLATFORM_DEFAULT_STACK_SIZE; }

      ...
      }
      ----------------------

        Activity

        Hide
        Arthur Naseef added a comment -

        Is the char[] allocation which causes the crash is in the AMQ-CPP library, then where is the array code in question? How big is the array? Do you have a stack trace?

        Show
        Arthur Naseef added a comment - Is the char[] allocation which causes the crash is in the AMQ-CPP library, then where is the array code in question? How big is the array? Do you have a stack trace?
        Hide
        omri zomet added a comment -

        The char[] allocation isn't in the AMQ-CPP library, but in the callback function onMessage() of my MessageListener.
        The array is of size 16384.
        I can post a stack trace - however not sure it will help, as it simply crashes in one of the sub-function being called (SIGSEGV).

        If I declare a bigger array (say 200,000) in onMessage() itself - I get a crash on calling onMessage() - see below:

        Program received signal SIGSEGV, Segmentation fault.
        [Switching to Thread 0x2ce1b90 (LWP 6128)]
        0x083c502a in CActiveMQClient::onMessage (this=0x8b5b5b0, message=<error reading variable>) at ActiveMQClient.cpp:703
        703 {

        Show
        omri zomet added a comment - The char[] allocation isn't in the AMQ-CPP library, but in the callback function onMessage() of my MessageListener. The array is of size 16384. I can post a stack trace - however not sure it will help, as it simply crashes in one of the sub-function being called (SIGSEGV). If I declare a bigger array (say 200,000) in onMessage() itself - I get a crash on calling onMessage() - see below: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x2ce1b90 (LWP 6128)] 0x083c502a in CActiveMQClient::onMessage (this=0x8b5b5b0, message=<error reading variable>) at ActiveMQClient.cpp:703 703 {
        Hide
        Arthur Naseef added a comment -

        I see. So, what are you looking for here? A way to change the stack size, or a wholesale increase in the default size? Keep in mind that large numbers of threads with large stack sizes can use up all available address space for the application, causing an out-of-memory condition.

        Can you use dynamic allocation with auto_ptr or unique_ptr to ensure deletion of the array at the right time?

        auto_ptr: http://www.cplusplus.com/reference/memory/auto_ptr/
        unique_ptr: http://www.cplusplus.com/reference/memory/unique_ptr/

        Something like this is what I'm proposing:

           void onMessage (Message *msg)
           {
             char[] data;
        
             data = new char[16384];
             unique_ptr  delete_data_on_leaving_scope(data);
        
             ...
        
             // Do NOT call delete on data.  It will be done by the unique_ptr under all code paths.
          }
        

        Note I've never personally used unique_ptr, only auto_ptr, so test this.

        Show
        Arthur Naseef added a comment - I see. So, what are you looking for here? A way to change the stack size, or a wholesale increase in the default size? Keep in mind that large numbers of threads with large stack sizes can use up all available address space for the application, causing an out-of-memory condition. Can you use dynamic allocation with auto_ptr or unique_ptr to ensure deletion of the array at the right time? auto_ptr: http://www.cplusplus.com/reference/memory/auto_ptr/ unique_ptr: http://www.cplusplus.com/reference/memory/unique_ptr/ Something like this is what I'm proposing: void onMessage (Message *msg) { char[] data; data = new char[16384]; unique_ptr delete_data_on_leaving_scope(data); ... // Do NOT call delete on data. It will be done by the unique_ptr under all code paths. } Note I've never personally used unique_ptr, only auto_ptr, so test this.
        Hide
        Timothy Bish added a comment -

        I believe we shouldn't be overriding the pthread defaults here unless a stack size is really given. This probably was left over from working on the Windows side and not cleaning up. Letting this default to the default would allow for control of all thread stacks via ulimit or calls to setrlimit() for RLIMIT_STACK mods.

        Show
        Timothy Bish added a comment - I believe we shouldn't be overriding the pthread defaults here unless a stack size is really given. This probably was left over from working on the Windows side and not cleaning up. Letting this default to the default would allow for control of all thread stacks via ulimit or calls to setrlimit() for RLIMIT_STACK mods.
        Hide
        Arthur Naseef added a comment - - edited

        That's a valid perspective.

        On the other hand, aren't these threads created by the activemq-cpp library itself? It's perfectly reasonable for the library to define the stack size in that case and make that part of the calling contract for onMessage().

        Another consideration is that many developers who are not used to programming with threads will easily run into problems setting the default stack too large. Or, setting it too low, the activemq-cpp library's needs may not be adequately met, leading to mysterious faults in the library itself.

        Threaded C/C++ programs with large automatic variables (allocated on the stack) are an antipattern. When working with libraries that make extensive use of exceptions, I see the desire to use them - that's why the standard C++ library has tools like auto_ptr and unique_ptr.

        Show
        Arthur Naseef added a comment - - edited That's a valid perspective. On the other hand, aren't these threads created by the activemq-cpp library itself? It's perfectly reasonable for the library to define the stack size in that case and make that part of the calling contract for onMessage(). Another consideration is that many developers who are not used to programming with threads will easily run into problems setting the default stack too large. Or, setting it too low, the activemq-cpp library's needs may not be adequately met, leading to mysterious faults in the library itself. Threaded C/C++ programs with large automatic variables (allocated on the stack) are an antipattern. When working with libraries that make extensive use of exceptions, I see the desire to use them - that's why the standard C++ library has tools like auto_ptr and unique_ptr.
        Hide
        Timothy Bish added a comment -

        Updated the default on 3.8.x to a slightly larger value. You will still need to use the heap if your dataset is larger. On trunk we now stick to sys defaults and let that be tuned by the user. Can still be set on user created threads via the Thread class constructor.

        Show
        Timothy Bish added a comment - Updated the default on 3.8.x to a slightly larger value. You will still need to use the heap if your dataset is larger. On trunk we now stick to sys defaults and let that be tuned by the user. Can still be set on user created threads via the Thread class constructor.

          People

          • Assignee:
            Timothy Bish
            Reporter:
            omri zomet
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development