Full_Name: Hallvard B Furuseth Version: 2.4.32 OS: Linux x86_64 URL: Submission from: (NULL) (193.69.163.163) Submitted by: hallvard
libmdb omits necessary checks for system call errors, and does not always clean up after the errors it does detect.
Also mdb could catch more user errors - including "cannot happen" mutex errors from mutex lock/unlock, since some threading user errors are be non-obvious from the doc.
At least sem_wait() needs an EINTR loop. POSIX says "The sem_wait() function is interruptible by the delivery of a signal." The Linux manpage adds that a signal handler always interrupts it regardless of sigaction SA_RESTART. I don't know about BSD.
Some error checks first need updates to the Windows emulation, or will look cleaner that way. E.g. pthread_key_create().
I'll fix mdb_env_open()'s error cleanup, at least. (Hyc, that moves cleanup out of mdb_env_setup_locks(). Any preference for using ~25 exit points 'return <ErrCode() or rc>' vs. ~25 'goto <failure label>' which does the same?)
Maybe add a flag to request error paranoia, like setting PTHREAD_MUTEX_ERRORCHECK. Trying to catch if a current mdb process has lost is locks. (The fcntl F_SETLK is lost if the process closes another file descriptor to the same file, e.g. after some other module in the process opened and closed the mdb lock file.) Catching a change to the pid from getpid().
Regarding the pid: Repeating getpid() - in mdb_txn_renew0() and mdb_env_close() - is useless since mdb cannot survive fork() anyway. fork drops fcntl locks. Might as well store the pid (or just some unique "environment id") in MDB_env and use that. OTOH some strategically placed getpid()s can then catch the user error of having forked, and set e.g. MDB_FATAL_ERROR.
The fork() restriction needs to be documented, as does "don't open or close the same database twice in the same process".
mdb_env_open() should catch being called twice. Except, should it try to allow a 2nd call if 1st mdb_env_open() failed? It can easily do that if 1st attempt failed before reading params from the DB files, but fail if the 1st open got that far.