On November 25, [EMAIL PROTECTED] said:
> We should probably clarify it with the dspam developers, but dspam
> seems to be currently stalled, for one reason or another.
>
> Any idea why?
If you mean upstream dspam development seems to be stalled, it seems
that way to me, too. Jonathan Zdziarski has been quiet lately on the
users and developers list as well. I think he may have picked up
another job or something. The only thing recently updated on his
personal website (http://www.zdziarski.com/) in months is his stock
picks.
In the meantime, i've written a short test program (attached below)
that tries to emulate the fcntl locking used by dspam, which shows
pretty clearly that it doesn't work between threads:
[0 [EMAIL PROTECTED] testlocking]$ ./testlocking 2>&1 | ts
Nov 25 10:54:39 Getting the lock in thread XX A
Nov 25 10:54:39 I've got the lock in thread XX A (fd: 3)
Nov 25 10:54:40 Getting the lock in thread XX B
Nov 25 10:54:40 I've got the lock in thread XX B (fd: 4)
Nov 25 10:54:41 Releasing the lock in thread XX A
Nov 25 10:54:41 Lock freed in thread XX A
Nov 25 10:54:41 Finishing thread XX A
Nov 25 10:54:42 Releasing the lock in thread XX B
Nov 25 10:54:42 Lock freed in thread XX B
Nov 25 10:54:42 Finishing thread XX B
[0 [EMAIL PROTECTED] testlocking]$
So if there's any control that keeps threads separate between
usernames, it's not being done successfully with fcntl locking.
i note that src/daemon.c seems to wrap each call to process_users in a
pthread_mutex_lock(), which might be an alternate (non-fcntl) method
of controlling access to the storage drivers. In particular, it locks
a pooled connection from thread context's storage driver's context
TTX->DTX->connections[i]->lock. Since the hash storage driver should
only have one pooled "connection" to the data, that might be
acceptable. I haven't audited the code completely to ensure that all
access to the hash driver happens within the lock, though.
However src/daemon.c also (outside the lock) calls initialize_atx()
from src/shared_agent.c. initialize_atx() can potentially call
exit(EXIT_FAILURE) if getpwnam_r() fails. Calling exit() (if i
understand correctly), would immediately kill all threads, including
any thread which might be holding the lock while accessing a hashdb,
which could result in corruption of that hashdb. This could be a
problem for any of the storage driver backends, actually, unless
they're using explicit transaction-wrapped database access.
once a hashdb is corrupted, the next time a dspam thread tries to
access it, the daemon itself may well crash, since the hashdb is an
mmap()'ed structure. If the daemon crashes, any other hashes open
concurrently may also become corrupt, leading to a cascading chain of
failures. yuck.
I haven't found any other instances of exit() that might be called
from the daemon, but i'm not sure that my audit was completely
exhaustive.
However we decide to handle the hash/daemon combination, i still think
it would be useful for the storage drivers to be able to report their
own threadsafety, and for the daemon to refuse to run if the report
was missing or negative.
--dkg
/* test program to see if we can use fcntl locking across threads, the
way dspam's hash driver tries to. */
#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>
#include <unistd.h>
#include <pthread.h>
#include <errno.h>
#include <string.h>
#define EFAILURE -1
const char * flockname = "fakelock";
/* ripped out of dspam's src/util.c: */
int _ds_get_fcntl_lock(int fd) {
#ifdef _WIN32
return 0;
#else
struct flock f;
f.l_type = F_WRLCK;
f.l_whence = SEEK_SET;
f.l_start = 0;
f.l_len = 0;
return fcntl(fd, F_SETLKW, &f);
#endif
}
int _ds_free_fcntl_lock(int fd) {
#ifdef _WIN32
return 0;
#else
struct flock f;
f.l_type = F_UNLCK;
f.l_whence = SEEK_SET;
f.l_start = 0;
f.l_len = 0;
return fcntl(fd, F_SETLKW, &f);
#endif
}
int getlock(FILE** x, const char* threadname) {
int r;
(*x) = fopen(flockname, "a");
if ((*x) == NULL) {
fprintf(stderr, "[%s] Failed to open file for appending for lock with error %d (%s).\n", threadname, errno, strerror(errno));
return EFAILURE;
}
r = _ds_get_fcntl_lock(fileno(*x));
if (r) {
fprintf(stderr, "[%s] Failed to get lock with error %d (%s)\n", threadname, r, strerror(errno));
fclose(*x);
}
return r;
}
int freelock (FILE** x, const char* threadname)
{
int r;
r = _ds_free_fcntl_lock(fileno(*x));
if (!r) {
fclose(*x);
} else {
fprintf(stderr, "[%s] Failed to free lock with error %d (%s)\n", threadname, r, strerror(errno));
}
return r;
}
void* threadA(void* x) {
FILE* f;
const char* threadname = x;
fprintf(stderr, "Getting the lock in thread %s\n", threadname);
if (0 == getlock(&f, threadname)) {
fprintf(stderr, "I've got the lock in thread %s (fd: %d)\n", threadname, fileno(f));
// we've got the lock
sleep(2);
fprintf(stderr, "Releasing the lock in thread %s\n", threadname);
if (0 == freelock(&f, threadname)) {
fprintf(stderr, "Lock freed in thread %s\n", threadname);
} else {
fprintf(stderr, "Failed to free lock in thread %s\n", threadname);
}
} else {
fprintf(stderr, "Failed to get lock in thread %s\n", threadname);
}
fprintf(stderr, "Finishing thread %s\n", threadname);
}
int main(int argc, char* argv[]) {
pthread_attr_t attr_a, attr_b;
pthread_t thread_a, thread_b;
char AName[256], BName[256];
const char* extraname = "XX";
if (argc > 1)
extraname = argv[1];
snprintf(AName, sizeof(AName), "%s A", extraname);
snprintf(BName, sizeof(BName), "%s B", extraname);
pthread_attr_init(&attr_a);
pthread_attr_setdetachstate(&attr_a, PTHREAD_CREATE_DETACHED);
pthread_create(&thread_a, &attr_a, threadA, (void *) AName);
sleep(1);
pthread_attr_init(&attr_b);
pthread_attr_setdetachstate(&attr_b, PTHREAD_CREATE_DETACHED);
pthread_create(&thread_b, &attr_b, threadA, (void *) BName);
sleep(5);
return 0;
}
