Full_Name: Leonid Yuriev Version: 2.4.40 OS: RHEL7 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (31.130.36.33) In continue of ITS#7969 and ITS#7970. An additional pages may be allocated unreasonable from map. Also the MDB_MAP_FULL error may returned while the space is really available. As we can see on lines 2500-2550 of mdb.c the field mr_txnid updated after the mr_pid, which is also is a flag of that reader's slot is occupied and other fields are valid, e.g. it is: ti->mti_readers[i].mr_pid = pid; /* setup reader's pid, and indicate that other fields is valid. / ti->mti_readers[i].mr_tid = tid; /* setup reader's thread id. */ ... /* do a little bit of something. / r->mr_txnid = ti->mti_txnid; / * update a "detent" for reclaiming. */ The problem is that not all cases, the value mr_txnid correctly before being updated, but will be counted immediately after setup pid in reader's slot. Valid value for mr_txnid can be any number not less than the last transaction to now. We can easily verify that the MRC may be incorrect adding mdb_eassert(env, r->mr_txnid >= ti->mti_txnid) after updating mti_numreaders. diff --git a/libraries/liblmdb/mdb.c b/libraries/liblmdb/mdb.c index 262373e..f2ed653 100644 --- a/libraries/liblmdb/mdb.c +++ b/libraries/liblmdb/mdb.c @@ -2522,6 +2522,7 @@ mdb_txn_renew0(MDB_txn *txn) return rc; } %%7} + mdb_eassert(env, r->mr_txnid >= ti->mti_txnid); do /* ITS# */ r->mr_txnid = ti->mti_txnid; And then by 'make test' got: #2 0x0000000000403722 in mdb_assert_fail (env=env@entry=0x134e010, expr_txt=expr_txt@entry=0x40fd14 "r->mr_txnid >= ti->mti_txnid", func=func@entry=0x41067f <__FUNCTION__.7150> "mdb_txn_renew0", line=line@entry=2525, file=0x40fd08 "mdb.c") at mdb.c:1347 buf = "mdb.c:2525: Assertion 'r->mr_txnid >= ti->mti_txnid' failed in mdb_txn_renew0()\000\000\000\000\000\000\000\000\000 \025\063\337\273\177\000\000\360\360\316\367\377\177\000\000\340\360\316\367\377\177\000\000\236,]\242\000\000\000\000x\n@\000\000\000\000\000\377\377\377\377\000\000\000\000\002\000\000\000\000\000\000\000\320\016C3C357\336\273\177\000\000\310\344\062\337\273\177\000\000\001\000\000\000\000\000\000\000\003", '\000' <repeats 15 times>, "M\000\000\000\000\000\000\000\003\000\000\000\000\000\000\000\016\000\000\000\000\000\000\000"... #3 0x0000000000403de8 in mdb_txn_renew0 (txn=txn@entry=0x134e210) at mdb.c:2525 r = 0x7fbbdf32b080 env = 0x134e010 ti = 0x7fbbdf32b000 meta = <optimised out> i = <optimised out> nr = <optimised out> x = <optimised out> rc = <optimised out> new_notls = 0 __FUNCTION__ = "mdb_txn_renew0" #4 0x00000000004045b8 in mdb_txn_begin (env=0x134e010, parent=parent@entry=0x0, flags=flags@entry=131072, ret=ret@entry=0x7ffff7cef2f0) at mdb.c:2706 txn = 0x134e210 ntxn = <optimised out> rc = <optimised out> size = <optimised out> tsize = 136 There is a possibility that the writer will call mdb_page_alloc() while the value of mr is wrong. In such case it is possible then reclaiming couldn't be done and a new page will be allocated from the map. But also possible the MDB_MAP_FULL error. Thus, because of this bug additional pages may be allocated unreasonable, and the MDB_MAP_FULL error may happen while the space is available.
The attached files is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned rights and/or interest in this work to any party. I, Leonid Yuriev am authorized by Peter-Service LLC, my employer, to release this work under the following terms. Peter-Service LLC hereby places the following modifications to OpenLDAP Software (and only these modifications) into the public domain. Hence, these modifications may be freely used and/or redistributed for any purpose with or without attribution and/or other notice. commit ff0bde6a95bb8ab2355324b65f451216d5ef2a0e Author: Leo Yuriev <leo@yuriev.ru> Date: 2014-10-18 02:00:37 +0400 ITS#7971 for LMDB: clarification in mdb_txn_renew0(). diff --git a/libraries/liblmdb/mdb.c b/libraries/liblmdb/mdb.c index 83a15c1..1e766af 100644 --- a/libraries/liblmdb/mdb.c +++ b/libraries/liblmdb/mdb.c @@ -2507,15 +2507,16 @@ mdb_txn_renew0(MDB_txn *txn) UNLOCK_MUTEX_R(env); return MDB_READERS_FULL; } - ti->mti_readers[i].mr_pid = pid; - ti->mti_readers[i].mr_tid = tid; + r = &ti->mti_readers[i]; + r->mr_txnid = (txnid_t)-1; + r->mr_tid = tid; + r->mr_pid = pid; /* should be written last, see ITS#. */ if (i == nr) ti->mti_numreaders = ++nr; /* Save numreaders for un-mutexed mdb_env_close() */ env->me_numreaders = nr; UNLOCK_MUTEX_R(env); - r = &ti->mti_readers[i]; new_notls = (env->me_flags & MDB_NOTLS); if (!new_notls && (rc=pthread_setspecific(env->me_txkey, r))) { r->mr_pid = 0;
The attached files is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned rights and/or interest in this work to any party. I, Leonid Yuriev am authorized by Peter-Service LLC, my employer, to release this work under the following terms. Peter-Service LLC hereby places the following modifications to OpenLDAP Software (and only these modifications) into the public domain. Hence, these modifications may be freely used and/or redistributed for any purpose with or without attribution and/or other notice. https://gist.github.com/leo-yuriev/6f1e1444c9e29bd94885
Committed to mdb.master > commit ff0bde6a95bb8ab2355324b65f451216d5ef2a0e > Author: Leo Yuriev <leo@yuriev.ru> > Date: 2014-10-18 02:00:37 +0400 > > ITS#7971 for LMDB: clarification in mdb_txn_renew0(). > > diff --git a/libraries/liblmdb/mdb.c b/libraries/liblmdb/mdb.c > index 83a15c1..1e766af 100644 > --- a/libraries/liblmdb/mdb.c > +++ b/libraries/liblmdb/mdb.c > @@ -2507,15 +2507,16 @@ mdb_txn_renew0(MDB_txn *txn) > UNLOCK_MUTEX_R(env); > return MDB_READERS_FULL; > } > - ti->mti_readers[i].mr_pid = pid; > - ti->mti_readers[i].mr_tid = tid; > + r = &ti->mti_readers[i]; > + r->mr_txnid = (txnid_t)-1; > + r->mr_tid = tid; > + r->mr_pid = pid; /* should be written last, see ITS#. */ > if (i == nr) > ti->mti_numreaders = ++nr; > /* Save numreaders for un-mutexed mdb_env_close() */ > env->me_numreaders = nr; > UNLOCK_MUTEX_R(env); > > - r = &ti->mti_readers[i]; > new_notls = (env->me_flags & MDB_NOTLS); > if (!new_notls && (rc=pthread_setspecific(env->me_txkey, r))) { > r->mr_pid = 0; > > > > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
fixed in mdb.master
changed notes changed state Open to Test moved from Incoming to Software Bugs
changed state Test to Release
On 18/10/14 00:46, leo@yuriev.ru wrote: > (...) > - ti->mti_readers[i].mr_pid = pid; > - ti->mti_readers[i].mr_tid = tid; > + r = &ti->mti_readers[i]; > + r->mr_txnid = (txnid_t)-1; > + r->mr_tid = tid; > + r->mr_pid = pid; /* should be written last, see ITS#. */ > if (i == nr) > ti->mti_numreaders = ++nr; > /* Save numreaders for un-mutexed mdb_env_close() */ > env->me_numreaders = nr; Actually that too is too early to set mr_pid: We must be sure env_close() will reset it. But it is also too late, we must get rid of any garbage value in md_pid at once to protect it from other processes' env_close(). Fixing. Also, I'll rename the confusingly named me_numreaders to me_close_readers and drop some confused code using it.
changed state Release to Closed