Strange concurrency issue with STL / OpenMP in 64 bit builds

Question

I have an odd issue when I build one of our projects in a 64 bit debug config. It appears to produce some strange behaviour which looks a little like a single iterator is getting incremented multiple times. I've narrowed it down to the following test code:

#include <omp.h>

#define _HAS_ITERATOR_DEBUGGING 0

#include <vector>
#include <set>

int main(int argc, const char* argv[]) {    
   std::vector<int> v;
   for(int j = 0; j < 20; ++j) {
      v.push_back(j);
   }

   #pragma omp parallel for
   for(int i = 0; i < 100000; ++i) {
      std::set<int> s;
      std::vector<int>::const_iterator begin = v.begin(), end = v.end();
      s.insert(begin, end); // after this line s should contain the numbers 0-19
      if(s.size() != 20) {
         printf("fail\n");
         exit(3);
      }
   }
   return 0;
}

However, the size test frequently fails which implies that somehow it is not inserting the entire contents of the vector - and a lot of poking at it makes it look rather like the vector iterators are being incremented more than one step at a time. It's quite hard to tell though since it doesn't tend to happen if one breaks in with the debugger.

The obvious conclusion to draw would be that it's not threadsafe, but my understanding is that it should be because the only variable modified is s which has local scope.

There are quite a few things which will fix the immediate problem:

• remove the parallel for
• throw a critical section around the insert() call
• #define HAS_ITERATOR_DEBUGGING 1
• replace the single insert() call with a manual loop and insert each item individually (this is basically what that function does internally, but the problem definitely goes away when I do it myself)
• build a 32bit version of the same code
• build the release version of the same code

This is being compiled under MSVC++ 2008 SP1, with the compiler-supplied STL implementation.

Can anyone shed any light on what's going on here? Thanks in advance for any hints - I'm pretty stumped :)

Edit: In case it is not clear, I am not looking for a "quick fix" to make this code work; as noted above I know of quite a few of those. What I want to understand is why this problem is occurring in the first place.

Edit 2: Code works correctly when compiled with gcc.

Answer 1

Looks like a bug in VS 2008 implementation of OpenMP and/or STL. With VS 2010, it is not reproduced; moreover it is not reproduced with Intel's compiler and VS 2008 headers, which makes me think that the bug is more likely in OpenMP support in VC++ compiler.

EDIT: The code with firstprivate I posted before does not work indeed, even with _SECURE_SCL=1. Seems I just had a single lucky run.

Answer 2

I agree with darhuk, I would suggest to mark s private

#pragma omp parallel for private(s)

Answer 3

I've never written any OpenMP code, but I have done a lot of multi-threaded programming, so my instincts may or may not be right.

The insert() method on the set is not thread safe, and it's failing because multiple threads are invoking it concurrently. You must lock (or some OpenMP equivalent) to guarantee the inserts are performed sequentially.

EDIT: Try this

#include <omp.h>

#define _HAS_ITERATOR_DEBUGGING 0

#include <vector>
#include <set>


int main(int argc, const char* argv[]) {    
   std::vector<int> v;
   for(int j = 0; j < 20; ++j) {
      v.push_back(j);
   }


omp_lock_t writelock;
OMP_INIT_LOCK(&writelock);

   #pragma omp parallel for
   for(int i = 0; i < 100000; ++i) {
      std::set<int> s;
      std::vector<int>::const_iterator begin = v.begin(), end = v.end();
      OMP_SET_LOCK(&writelock);
      s.insert(begin, end); // after this line s should contain the numbers 0-19
      OMP_UNSET_LOCK(&writelock);
      if(s.size() != 20) {
         printf("fail\n");
         exit(3);
      }
   }
   return 0;
}