search

includeos / IncludeOS

A minimal, resource efficient unikernel for cloud services
https://includeos.github.io/
Apache License 2.0
4.93k stars 365 forks source link

Possible data race in src/posix/pthread.cpp #2200

Closed mfbalin closed 4 years ago

mfbalin commented 4 years ago

Inside pthread_create, static int __thread_id__ = 0 increments each time a new thread is created, and it is not atomic, possibly causing a data race.

fwsGonzo commented 4 years ago

Not saying you are wrong here because there IS a race inside my threads implementation, but are you sure about this? Because I'm incrementing using:

  inline long generate_new_thread_id() noexcept {
      return __sync_fetch_and_add(&thread_counter, 1);
  }

So, regardless of the counter itself, the source of new IDs is an atomic add.

mfbalin commented 4 years ago 
static int __thread_id__ = 0;
auto new_id = __thread_id__++;

This is what I see on line 16.

fwsGonzo commented 4 years ago

Oh yes, I thought you meant src/kernel/threads.cpp. No, we don't use that anymore - it should have been deleted. We get pthreads from musl now.

https://github.com/includeos/IncludeOS/blob/master/src/musl/CMakeLists.txt

The file isn't in the build list.

mfbalin commented 4 years ago

Alright, thanks. I was glancing over the code and thought maybe I should open an issue about this.