[Date Prev][Date Next]
(ITS#7377) Poor libmdb error handling
Full_Name: Hallvard B Furuseth
OS: Linux x86_64
Submission from: (NULL) (220.127.116.11)
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.