Re: Bug in C++ STL crashes on different SMP architectures

[Frederic Olivie]

> Frederic Olivie wrote:
> >
> > Hi,
> >
> > In November 2000, I found a post from a guy named maciek Kozyrczak :
> >
> > http://mail.gnu.org/pipermail/bug-glibc/2000-November/002118.html
> >
> > We just ran into the same exact problem.
> >
> > The basic_string set of templates is NOT mt_safe.
> >
>
> Have you read the design document for SGI STL? You should know
> that making the container classes mt_safe in some idiot proof
> way would yield a severe performance penalty.
>
> Now, send a minimal piece of code so that I can comment on it.
> In particular, only the memory allocation is synced by default,
> except that you will have to use synchronization constructs to capture
> application semantics. This isn't Java.

Yes, we have. And we are totally aware that protecting each and every piece
of code against bad usage of concurrent accesses would definitely be too
slow.

But, in this specific case, the problem is coming from the implementation of
the reference count of basic_string (ref).
This ref count is incremented by one (line 75 of my bastring.h) without any
protection against concurrent accesses.
This yields to a weird behavior on the struct itself, usually setting the
len field to a huge value. Thus breaking up the class itself, and usually
leading to a segfault when the destructor is called.

A good and simple example of this has been given by Maciek Kozyrczak. You
will find it at the bottom of this post.

We have reproduced this bug (actually, suffered from it) on two different
SMP platforms (Linux/i386 and AIX/RS6000). SMP is required as on
uniprocessor architectures, either it happens very rarely, or the threads
can't really work simultaneously on the same structure.

So, it looks like the only problem is coming from the static nilRep data
structure which is not protected against concurrent accesses.
One solution would be protecting all string accesses in our code, but this
looks quite heavy.

Maciek offered us also a temporary one, which would be replacing the false
value of the selfish boolean by true. This would lead to not using some of
the optimizations of the template, but it looks like it might solve our
problem.


Code submitted by Maciek Kozyrczak :

#define _REENTRANT
#define _THREAD_SAFE
#define _PTHREADS
#define _POSIX_THREADS
#define _POSIX_THREAD_SAFE_FUNCTIONS

#include <cerrno>
#include <string>
using namespace std;

extern "C" {
#include <pthread.h>
#include <time.h>
}

// Lock for cerr
pthread_mutex_t cerr_lock = PTHREAD_MUTEX_INITIALIZER;

void *
handler(void *args)
{
  int thr_num = (int)args;

  pthread_mutex_lock(&cerr_lock);
  cerr << "Thread #" << thr_num << " starting" << endl;
  pthread_mutex_unlock(&cerr_lock);

  //  pthread_mutex_t sleep_lock = PTHREAD_MUTEX_INITIALIZER;
  //  pthread_cond_t sleep_cond = PTHREAD_COND_INITIALIZER;

  while (1) {
    string foo;

    if (foo.length() != 0) {
      pthread_mutex_lock(&cerr_lock);
      cerr << "Thread #" << thr_num << ": length = " << foo.length() <<
endl;
      pthread_mutex_unlock(&cerr_lock);
      exit (1);
    }
  }

  return (void*)0;
}

int
main(int argc, char *argv[])
{
  int i;

  // Fork some threads.
  // The total number of threads running in the program is going to be
4.
  for (i = 0; i < 99; i) {
    // Fork a child thread to execute the work of the handler
    pthread_t child_thread;
    while (1) {
      pthread_attr_t child_attr;
      pthread_attr_init(&child_attr);
      pthread_attr_setdetachstate(&child_attr,PTHREAD_CREATE_DETACHED);
      int rc =
pthread_create(&child_thread,&child_attr,handler,(void*)i);
      if (rc == 0) break;
      if (rc == EAGAIN) continue;
      // Error in the thread fork.
      cerr << __FILE__ << ":" << __LINE__ << ": Error forking thread" <<
endl;
      exit(1);
    }
    pthread_mutex_lock(&cerr_lock);
    cerr << "Thread #" << i << " forked" << endl;
    pthread_mutex_unlock(&cerr_lock);
  }

  // Get the main thread executing the same handler function.
  handler((void*)i);

  // Sanity check
  cerr << "Done with main()" << endl;

  return 0;
}