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

Re: (ITS#7364) mdb: clean up POSIX semaphores on environment close.



Howard Chu writes:
> h.b.furuseth@usit.uio.no wrote:
>> Reopening this.
>> 
>> 
>> This is worse with a database with is intended to be used by
>> different users (A and B):
> 
> This is pretty much never a use case we would worry about. In most
> applications, a single userID creates and operates on the DB.

I'm fine with just documenting that as a restriction on some systems.

If not:

>> (...)
>> The work-around I can think of is a "multi-uid" mode which instead
>> resets the semaphore with sem_post() if sem_getvalue() returns 0.
>> I don't know how ugly that is considered to be.  Could ask
>> comp.programming.unix, or check what Berkeley DB does.
> 
> That would be OK in general. It still leaves the question of how to remove the
> semaphore if the DB is being destroyed. But it's probably not worth the
> trouble to worry about this so much. Those OSes should just get their act
> together and support the POSIX process-shared mutexes.
> 
>> This mode
>> should use mode 0666 for the semaphores (temporarily setting umask
>> 0, yuck),
> 
> Definitely not. The caller specifies a mode; if they want 0666 they
> should configure it as such.

0666 would likely be wrong for the database file(s).  But this flag
could just as well consist of specifying a mode parameter for the
semaphores.  It's a threaded library doing umask() I dislike.
And, as you say, the need to remove the semaphore afterwards.

>> or it should not sem_unlink() since next user may create
>> the semaphores with a group which gives the wrong users access.
> 
> Same as today where running slapadd with the wrong uid causes trouble
> for the following slapd. The answer is obvious: use the right uid when
> accessing the DB.

We're talking about the case where there is no single "right" uid.
Not relevant for slapd, only libmdb by itself.

>> Other matters with the current implementation - I'll patch these:
>> 
>> mdb_env_excl_lock() need not retry getting a non-exclusive lock when
>> closing.  mdb_env_close() can pass *excl = -1 to tell it not to.
>> 
>> mdb_env_setup_locks() can sem_unlink both semaphores before doing
>> anything else, so that reopening a database as root will clean up.
>> Drop the error checks of sem_unlink (so both get called), instead
>> use O_EXCL in sem_open(,O_CREAT,,).  Unless I'm missing something,
>> the error checks just work like an emulation of O_EXCL anyway.
> 
> The sem_unlink() and sem_open() sequence isn't ideal, certainly. I would
> prefer to just use the existing semaphore.

...followed by 'if (sem_getvalue() shows 0) sem_post()' as above, then.

-- 
Hallvard