Snaipe / libcsptr

Smart pointers for the (GNU) C programming language
https://snai.pe/c/c-smart-pointers/
MIT License
1.57k stars 143 forks source link

atomic_add impl report error by ThreadSanitizer #28

Open moonlightsh opened 9 months ago

moonlightsh commented 9 months ago

hello, I learn a lot from you project, When I write a multi-thread program and test. I use tsan to check data race, reports error:

0x7b0c00000328
==================
WARNING: ThreadSanitizer: data race (pid=48322)
  Read of size 8 at 0x7b0c00000318 by thread T1:
    #0 sref mman.c:91 (a.out:x86_64+0x100002c47)
    #1 thr main.c:16 (a.out:x86_64+0x100002655)
old_count: 2
  Previous atomic write of size 8 at 0x7b0c00000318 by main thread:
    #0 sref mman.c:91 (a.out:x86_64+0x100002c95)
    #1 main main.c:31 (a.out:x86_64+0x1000027a6)
old_count: 2
  Location is heap block of size 48 at 0x7b0c00000300 allocated by main thread:
    #0 malloc <null>:188732607 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x55a7c)
    #1 smalloc_impl mman.c:163 (a.out:x86_64+0x10000349d)
    #2 smalloc mman.c:212 (a.out:x86_64+0x100003341)
    #3 main main.c:26 (a.out:x86_64+0x1000026ed)

  Thread T1 (tid=486984, running) created by main thread at:
    #0 pthread_create <null>:188732607 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x3155f)
    #1 main main.c:30 (a.out:x86_64+0x100002794)

there is a simple demo just for test:

#include <stdio.h>
#include <pthread.h>
#include <assert.h>
#include <csptr/smart_ptr.h>

struct T {
    int foo;
};

static void cleanupfunc(void *ptr, void *meta) {
    printf("cleanupfunc\n");
}

void *thr(void *p)
{
    sref(p);
    sfree(p);
    return NULL;
}

int main()
{
    pthread_t t1;
    pthread_t t2;
    pthread_t t3;
    smart struct T *p = shared_ptr(struct T, {}, cleanupfunc);

    sref(p);
    pthread_create(&t1, NULL, thr, p);
    sref(p);
    pthread_create(&t2, NULL, thr, p);
    sref(p);
    pthread_create(&t3, NULL, thr, p);
    pthread_join(t1, NULL);
    pthread_join(t2, NULL);
    pthread_join(t3, NULL);
    sfree(p);
    sfree(p);
    sfree(p);
}

I check the code, found maybe the non-sync access for count , https://github.com/Snaipe/libcsptr/blob/ac734512f97d42be728c457b8084ae5fd6ef50c6/src/mman.c#L43-L52 But in think it not a real issue ,unless in x64. I got a way to avoid from abort of tsan, is it just make it happier?

static CSPTR_INLINE size_t atomic_add(volatile size_t *count, const size_t limit, const size_t val) {
    size_t old_count, new_count;
    do {
      old_count = __sync_fetch_and_add(count, 0);
      if (old_count == limit)
          abort();
      new_count = old_count + val;
    } while (!__sync_bool_compare_and_swap(count, old_count, new_count));
    return new_count;
}

also commit : 983932