#3 pthread error on NetBSD (Patch provided)

open
None
8
2008-01-07
2008-01-06
Christian Gall
No

In function put_into() the mutex flows_mutex[h] may be unlocked twice.

Unlocking a mutex twice is undefined behavior, on some platforms this may abort the program (for example on NetBSD).

Since the first unlock seems not to be necessary, the following patch disables this.

Discussion

  • Christian Gall
    Christian Gall
    2008-01-06

    • priority: 5 --> 8
    • summary: pthread error on NetBSD --> pthread error on NetBSD (Patch provided)
     
  • Christian Gall
    Christian Gall
    2008-01-07

    Logged In: YES
    user_id=1249624
    Originator: YES

    Patch does not work.

    The following should do it:

    --- src/fprobe.c.orig 2008-01-07 11:31:05.000000000 +0100
    +++ src/fprobe.c
    @@ -561,6 +561,7 @@ int put_into(struct Flow *flow, int flag
    , logbuf
    #endif
    );
    + pthread_mutex_lock(&flows_mutex[h]);
    }
    }
    if (flag == MOVE_INTO) mem_free(flow);

     
  • Christian Gall
    Christian Gall
    2008-01-07

    • assigned_to: nobody --> sla
     
  • Logged In: YES
    user_id=139903
    Originator: NO

    Thanks for the note. I missed the first patch, did you tried PTHREAD_MUTEX_ERRORCHECK type?

    --- fprobe.c 2005-02-15 21:12:36.000000000 +0300
    +++ fprobe.new.c 2008-01-08 21:38:14.931923834 +0300
    @@ -1456,7 +1456,7 @@

    hash_init(); /* Actually for crc16 only */
    mem_init(sizeof(struct Flow), bulk_quantity, memory_limit);
    - for (i = 0; i < 1 << HASH_BITS; i++) pthread_mutex_init(&flows_mutex[i], 0);
    + for (i = 0; i < 1 << HASH_BITS; i++) pthread_mutex_init(&flows_mutex[i], PTHREAD_MUTEX_ERRORCHECK);

    #ifdef UPTIME_TRICK
    /* Hope 12 days is enough :-/ */

     
  • Christian Gall
    Christian Gall
    2008-01-09

    Logged In: YES
    user_id=1249624
    Originator: YES

    I've deleted the first patch, since it was wrong and not doing the right thing.

    I haven't checked PTHREAD_MUTEX_ERRORCHECK. If I'm not wrong, PTHREAD_MUTEX_ERRORCHECK will return an error if a mutex is unlocked twice.

    On NetBSD, the program will be terminated.This behaviour can be disabled with PTHREAD_DIAGASSERT (see http://netbsd.gw.com/cgi-bin/man-cgi?pthread++NetBSD-4.0\)

    Using my patch seems to correct the problem, fprobe is running for two days now without problems.

     
  • Christian Gall
    Christian Gall
    2008-01-11

    Logged In: YES
    user_id=1249624
    Originator: YES

    I've tested it with the following patch:

    --- src/fprobe.c.orig 2008-01-11 22:26:29.000000000 +0000
    +++ src/fprobe.c 2008-01-11 22:40:18.000000000 +0000
    @@ -9,6 +9,8 @@

    #include <common.h>

    +#include <err.h>
    +
    /* stdout, stderr, freopen() */
    #include <stdio.h>

    @@ -470,6 +472,7 @@
    #endif
    )
    {
    + int ret_mutex = 0;
    int ret = 0;
    hash_t h;
    struct Flow *flown, **flowpp;
    @@ -546,7 +549,9 @@
    && (flown->sizeF >= flown->sizeP)) {
    /* All fragments received - flow reassembled */
    *flowpp = flown->next;
    - pthread_mutex_unlock(&flows_mutex[h]);
    + if ((ret_mutex = pthread_mutex_unlock(&flows_mutex[h])) != 0) {
    + warn("(a) error unlocking mutex. ret_mutex: %d", ret_mutex);
    + }
    #if ((DEBUG) & DEBUG_I)
    flows_total--;
    flows_fragmented--;
    @@ -565,7 +570,9 @@
    }
    if (flag == MOVE_INTO) mem_free(flow);
    }
    - pthread_mutex_unlock(&flows_mutex[h]);
    + if ((ret_mutex = pthread_mutex_unlock(&flows_mutex[h])) != 0) {
    + warn("(b) error unlocking mutex. ret_mutex: %d", ret_mutex);
    + }
    return ret;
    }

    @@ -1189,6 +1196,7 @@
    struct sigaction sigact;
    static void *threads[THREADS - 1] = {&emit_thread, &scan_thread, &unpending_thread, &pcap_thread};
    struct timeval timeout;
    + pthread_mutexattr_t attr;

    #ifdef WALL
    link_type_idx = 0;
    @@ -1456,7 +1464,10 @@

    hash_init(); /* Actually for crc16 only */
    mem_init(sizeof(struct Flow), bulk_quantity, memory_limit);
    - for (i = 0; i < 1 << HASH_BITS; i++) pthread_mutex_init(&flows_mutex[i], 0);
    +// for (i = 0; i < 1 << HASH_BITS; i++) pthread_mutex_init(&flows_mutex[i], 0);
    + pthread_mutexattr_init(&attr);
    + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);
    + for (i = 0; i < 1 << HASH_BITS; i++) pthread_mutex_init(&flows_mutex[i], &attr);

    #ifdef UPTIME_TRICK
    /* Hope 12 days is enough :-/ */

    $ src/fprobe -p -fip -l 2 -v 7 127.0.0.1:9996/127.0.0.1/
    [INFO]: Starting 1.1...
    [INFO]: pid: 25959
    [INFO]: interface: vlan2, datalink: EN10MB (1)
    [INFO]: filter: "ip"
    [INFO]: options: p=0 s=5 g=30 d=60 e=300 n=5 a=0.0.0.0 x=0:0 b=10000 m=0 q=100 B=0 r=0 t=0:0 S=256 K=14 k=0 c= u= v=7 l=2
    [INFO]: collector #1: 127.0.0.1:9996/127.0.0.1/m
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0
    fprobe: (b) error unlocking mutex. ret_mutex: 1: Undefined error: 0

     
  • Logged In: YES
    user_id=139903
    Originator: NO

    Thanks for your effort.
    I thiks the best solution is your variant with additional pthread_mutex_lock(). In short time I'll release bugfixed version.
    Thank you again.