Re: (ITS#7853) Improve safety of mdb_env_close

[hbf@mail.uio.no]

On 05/14/2014 01:18 AM, hyc@symas.com wrote:
> dw@botanicus.net wrote:
>> Currently if a user (wilfully or accidentally, say, through composition of
>> third party libs) opens the same LMDB environment twice within a process, and
>> maintains active read transactions at the time one mdb_env_close() is called,
>> all reader slots will be deallocated in all environments due to the logic
>> around line 4253 that unconditionally clears reader slots based on PID.
>>
>> I'd like to avoid this in py-lmdb, and seems there are a few alternatives to
>> fix it:
>>
>> 1) Tell users not to do that (doc fix, nobody reads docs)
>
> This is already in the doc - don't open the same env twice within one process.

1.5) Use something else than fcntl locks on the lock-file descriptor
to control whether to re-init the lockfile and its mutexes/semaphores.
Like MDB_NOLOCK.

Needed to get any other suggestions off the ground, see Caveats
in lmdb.h.  Once two MDB_env's in a process has the lockfile open,
both lose their fcntl locks when either closes it - and the next
mdb_env_open() will reset the lockfile.  Then both readers and
writers are screwed, and more care with reader slots won't help.

Well, one MDB_env could leak its lockfile descriptor instead of
closing it, or it could give the descriptor to the other MDB_env
for a deferred close if it could find it using some global state.
(Could work until someone wrote an open-close loop which would
run into the max-open-FD limit.)

Either way, no fun.

Another way would be #ifdef ... more unportable code ... #endif:-(
Surely some OSes have come up with sane way to address this
problem, there's just no portable way.

>> 2) Externally maintain a set of (st_dev, st_ino) keys representing already
>> opened environments, and check any attempt to open an environment against this
>> list, failing if data.mdb's key already appears.
>
> That would require global library state, which is definitely unwanted.

Though in one sense we already have that, which is the problem.
Global (well, per-process) lock state on open files and mutexes.

>> 3) Disable the mdb_env_close() cleanup loop when MDB_NOTLS is enabled. Since no
>> reader slot will ever exist in TLS in this mode, the existing mdb_env_close()
>> doc "All transactions, databases, and cursors must already be closed before
>> calling this function." ensures no readers exist at close time, and the loop
>> need not run.
>
> Could do this, but since it's an incomplete solution, not sure there's any point.

That's MDB_NOLOCK anyway.  MDB_NOTLS does have reader table entries.

 > (...)

-- 
Hallvard