#3 pthread error on NetBSD (Patch provided)

[Christian Gall]

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.

[Christian Gall]

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);

[Slava Astashonok]

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]

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]

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

[Slava Astashonok]

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.