[Date Prev][Date Next] [Chronological] [Thread] [Top]

(ITS#7377) Poor libmdb error handling



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.