https://bugs.openldap.org/show_bug.cgi?id=9278
Issue ID: 9278 Summary: liblmdb: robust mutexes should not be unmapped Product: LMDB Version: unspecified Hardware: All OS: FreeBSD Status: UNCONFIRMED Severity: normal Priority: --- Component: liblmdb Assignee: bugs@openldap.org Reporter: delphij@freebsd.org Target Milestone: ---
Created attachment 736 --> https://bugs.openldap.org/attachment.cgi?id=736&action=edit A possible workaround
We recently noticed that lmdb would have the memory region containing the robust mutex unmapped on mdb_env_close0():
munmap((void *)env->me_txns, (env->me_maxreaders-1)*sizeof(MDB_reader)+sizeof(MDB_txninfo));
Note that if this is the last unmap for a robust mutex, the FreeBSD implementation would garbage-collect the mutex, making it no longer visible to other processes. As the result, a second instance of the attached test.c (from https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=244493 with minor changes) would trigger the assertion at mdb_txn_begin() because the acquisition of the mutex would return 22 (EINVAL), because the mutex appeared to be a robust mutex, but was invalid.
The attached lmdb.diff is a possible workaround for this (it would skip unmapping when setting up the robust mutex for the first time).
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #1 from Xin Li delphij@freebsd.org --- Created attachment 737 --> https://bugs.openldap.org/attachment.cgi?id=737&action=edit Test case
This test case is derived from the one in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=244493 to demostrate the issue.
Running a second instance of the test program before the sleep in first one ends would trigger the assertion.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #2 from Howard Chu hyc@openldap.org --- Please specify the version of liblmdb you were using.
(In reply to Xin Li from comment #0)
The attached lmdb.diff is a possible workaround for this (it would skip unmapping when setting up the robust mutex for the first time).
The patch is clearly invalid. The region must be unmapped when the environment is closed. The same would happen anyway, if the process were to exit.
Once the last process closes the environment, all of the contents of the lockfile become invalid anyway, so the FreeBSD cleanup is the expected and correct action. On the next call to env_open, when we see that no other process has it open already, the lockfile is reinitialized and those mutexes are completely created again. If a second process is coming in at this point and getting EINVAL on those mutexes, that means that the mutex initialization screwed up somehow, which sounds like a FreeBSD bug.
https://bugs.openldap.org/show_bug.cgi?id=9278
Xin Li delphij@freebsd.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #736 is|0 |1 obsolete| |
--- Comment #3 from Xin Li delphij@freebsd.org --- Created attachment 738 --> https://bugs.openldap.org/attachment.cgi?id=738&action=edit Proposed patch
Oops, yes you are right and I clearly have some misunderstanding of the code here, sorry for the noise.
Here is a different proposal, basically, it would destroy the robust mutex if we are the only remaining user. My understanding of https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#ta... was that the effect after unmapping of the mutex object is undefined, on FreeBSD, it seems to confuse the threading library because after the last mmap is gone, the kernel GC's the object, while it's still on userland threading library's bookkeeping ( https://svnweb.freebsd.org/base/head/lib/libthr/thread/thr_pshared.c?annotat... ) if they are not destroyed.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #4 from Howard Chu hyc@openldap.org --- (In reply to Xin Li from comment #3)
Created attachment 738 [details] Proposed patch
Oops, yes you are right and I clearly have some misunderstanding of the code here, sorry for the noise.
Here is a different proposal, basically, it would destroy the robust mutex if we are the only remaining user. My understanding of https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02. html#tag_15_09_09 was that the effect after unmapping of the mutex object is undefined, on FreeBSD, it seems to confuse the threading library because after the last mmap is gone, the kernel GC's the object, while it's still on userland threading library's bookkeeping ( https://svnweb.freebsd.org/base/head/lib/libthr/thread/thr_pshared. c?annotate=297141#l220 ) if they are not destroyed.
Last time I checked, FreeBSD didn't even support robust process shared mutexes. What OS revision is required for this to be supported?
It seems to me that doing what this test program is doing is a misuse of the API, you should only ever open an environment once in any particular process. Closing it and then opening it again makes no sense.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #5 from Xin Li delphij@freebsd.org --- (In reply to Howard Chu from comment #4)
(In reply to Xin Li from comment #3)
Created attachment 738 [details] Proposed patch
Oops, yes you are right and I clearly have some misunderstanding of the code here, sorry for the noise.
Here is a different proposal, basically, it would destroy the robust mutex if we are the only remaining user. My understanding of https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02. html#tag_15_09_09 was that the effect after unmapping of the mutex object is undefined, on FreeBSD, it seems to confuse the threading library because after the last mmap is gone, the kernel GC's the object, while it's still on userland threading library's bookkeeping ( https://svnweb.freebsd.org/base/head/lib/libthr/thread/thr_pshared. c?annotate=297141#l220 ) if they are not destroyed.
Last time I checked, FreeBSD didn't even support robust process shared mutexes. What OS revision is required for this to be supported?
The support of robust mutexes was added in FreeBSD r300043 (May 17, 2016) and is supported by all currently supported FreeBSD releases (11.3, 11.4 and 12.1). The first release with robust mutex support was FreeBSD 11.0-RELEASE (October 2016).
It seems to me that doing what this test program is doing is a misuse of the API, you should only ever open an environment once in any particular process. Closing it and then opening it again makes no sense.
Without cleaning up the mutex before unmapping, lmdb will leave the recorded mutex behind and the current code will not be able to recover from that (unlike in the case the calling process crashed, the code would correctly recover from the situation). Since lmdb is performing similar cleanups for semaphores, destroying the mutexes when we are the last user of the database seems to be the right thing to do in my opinion.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #6 from Howard Chu hyc@openldap.org --- (In reply to Xin Li from comment #5)
(In reply to Howard Chu from comment #4)
(In reply to Xin Li from comment #3)
Created attachment 738 [details]
Last time I checked, FreeBSD didn't even support robust process shared mutexes. What OS revision is required for this to be supported?
The support of robust mutexes was added in FreeBSD r300043 (May 17, 2016) and is supported by all currently supported FreeBSD releases (11.3, 11.4 and 12.1). The first release with robust mutex support was FreeBSD 11.0-RELEASE (October 2016).
Is there a macro we can test, that includes the FreeBSD release number?
It seems to me that doing what this test program is doing is a misuse of the API, you should only ever open an environment once in any particular process. Closing it and then opening it again makes no sense.
Without cleaning up the mutex before unmapping, lmdb will leave the recorded mutex behind and the current code will not be able to recover from that (unlike in the case the calling process crashed, the code would correctly recover from the situation). Since lmdb is performing similar cleanups for semaphores, destroying the mutexes when we are the last user of the database seems to be the right thing to do in my opinion.
Yeah, that sounds ok.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #7 from Xin Li delphij@freebsd.org --- (In reply to Howard Chu from comment #6)
(In reply to Xin Li from comment #5)
(In reply to Howard Chu from comment #4)
(In reply to Xin Li from comment #3)
Created attachment 738 [details]
Last time I checked, FreeBSD didn't even support robust process shared mutexes. What OS revision is required for this to be supported?
The support of robust mutexes was added in FreeBSD r300043 (May 17, 2016) and is supported by all currently supported FreeBSD releases (11.3, 11.4 and 12.1). The first release with robust mutex support was FreeBSD 11.0-RELEASE (October 2016).
Is there a macro we can test, that includes the FreeBSD release number?
Yes, normally we would check __FreeBSD_version which is defined in sys/param.h.
The feature was introduced in r300043, but unfortunately __FreeBSD_version was not bumped at the time, so we can use the value in r300207 which is the first bump after that revision. In the code that would be something like:
#if defined(__FreeBSD_version) && __FreeBSD_version >= 1100110
(Note that this is really old version of FreeBSD, all non-EoL'ed FreeBSD releases supports this feature).
https://bugs.openldap.org/show_bug.cgi?id=9278
Howard Chu hyc@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution|--- |TEST
--- Comment #8 from Howard Chu hyc@openldap.org --- (In reply to Xin Li from comment #7)
(In reply to Howard Chu from comment #6)
(In reply to Xin Li from comment #5)
(In reply to Howard Chu from comment #4)
(In reply to Xin Li from comment #3)
Created attachment 738 [details]
Last time I checked, FreeBSD didn't even support robust process shared mutexes. What OS revision is required for this to be supported?
The support of robust mutexes was added in FreeBSD r300043 (May 17, 2016) and is supported by all currently supported FreeBSD releases (11.3, 11.4 and 12.1). The first release with robust mutex support was FreeBSD 11.0-RELEASE (October 2016).
Is there a macro we can test, that includes the FreeBSD release number?
Yes, normally we would check __FreeBSD_version which is defined in sys/param.h.
The feature was introduced in r300043, but unfortunately __FreeBSD_version was not bumped at the time, so we can use the value in r300207 which is the first bump after that revision. In the code that would be something like:
#if defined(__FreeBSD_version) && __FreeBSD_version >= 1100110
(Note that this is really old version of FreeBSD, all non-EoL'ed FreeBSD releases supports this feature).
Thanks. Patched in mdb.master and mdb.RE/0.9, please test.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #9 from Quanah Gibson-Mount quanah@openldap.org --- mdb.master
Commits: • 2fd44e32 by Howard Chu at 2020-06-16T19:49:14+01:00 ITS#9278 fix robust mutex cleanup for FreeBSD
FreeBSD 11 supports robust process-shared POSIX mutexes, but requires them to be explicitly destroyed before munmap
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #10 from Quanah Gibson-Mount quanah@openldap.org --- mdb.RE/0.9
Commits: • f683ffdc by Howard Chu at 2020-06-16T19:56:16+01:00 ITS#9278 fix robust mutex cleanup for FreeBSD
FreeBSD 11 supports robust process-shared POSIX mutexes, but requires them to be explicitly destroyed before munmap
• f681a076 by Howard Chu at 2020-06-16T19:57:41+01:00 Silence stupid fallthru warning
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #11 from Xin Li delphij@freebsd.org --- (In reply to Howard Chu from comment #8)
(In reply to Xin Li from comment #7)
(In reply to Howard Chu from comment #6)
(In reply to Xin Li from comment #5)
(In reply to Howard Chu from comment #4)
(In reply to Xin Li from comment #3)
Created attachment 738 [details]
Last time I checked, FreeBSD didn't even support robust process shared mutexes. What OS revision is required for this to be supported?
The support of robust mutexes was added in FreeBSD r300043 (May 17, 2016) and is supported by all currently supported FreeBSD releases (11.3, 11.4 and 12.1). The first release with robust mutex support was FreeBSD 11.0-RELEASE (October 2016).
Is there a macro we can test, that includes the FreeBSD release number?
Yes, normally we would check __FreeBSD_version which is defined in sys/param.h.
The feature was introduced in r300043, but unfortunately __FreeBSD_version was not bumped at the time, so we can use the value in r300207 which is the first bump after that revision. In the code that would be something like:
#if defined(__FreeBSD_version) && __FreeBSD_version >= 1100110
(Note that this is really old version of FreeBSD, all non-EoL'ed FreeBSD releases supports this feature).
Thanks. Patched in mdb.master and mdb.RE/0.9, please test.
I've build with mdb.master (2fd44e325195ae81664eb5dc36e7d265927c5ebc) on FreeBSD 13.0-CURRENT and it have corrected the issue. 'make test' in lmdb also seems to be working fine, thanks!
https://bugs.openldap.org/show_bug.cgi?id=9278
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |0.9.26
https://bugs.openldap.org/show_bug.cgi?id=9278
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|TEST |FIXED
https://bugs.openldap.org/show_bug.cgi?id=9278
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |VERIFIED
https://bugs.openldap.org/show_bug.cgi?id=9278
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.openldap.org/s | |how_bug.cgi?id=10058
https://bugs.openldap.org/show_bug.cgi?id=9278
Howard Chu hyc@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.openldap.org/s | |how_bug.cgi?id=10095 Resolution|FIXED |--- Ever confirmed|0 |1 Status|VERIFIED |CONFIRMED
--- Comment #12 from Howard Chu hyc@openldap.org --- We're going to revert this patch due to all the new race conditions it introduces (see other ITSs noted above). No other platforms have the problem reported here, so we must consider it a bug in FreeBSD that munmapping a mutex doesn't automatically perform all of the necessary libc cleanup itself.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #13 from Howard Chu hyc@openldap.org --- Must also note - there would still be another race condition, regardless:
When the first process destroys the mutexes, and still has the lockfile open, another process attempting to open the env during that time will see that the lockfile is locked, which it would take to mean all of the lockfile contents are still valid. Meanwhile the first process unmaps the region, and the kernel wipes out the underlying mutex structures. Then the first process closes the lockfile which releases the lock, the later process acquires the readlock and tries to use the lockfile as-is but its mutexes are no longer valid.
This race would occur regardless of what libc / libthr does since the kernel automatically GCs the underlying mutex structures as soon as the region is unmapped. It could be avoided if the kernel additionally waits to cleanup until there are no open descriptors on the file.
https://bugs.openldap.org/show_bug.cgi?id=9278
--- Comment #14 from Xin Li delphij@freebsd.org --- (In reply to Howard Chu from comment #12)
We're going to revert this patch due to all the new race conditions it introduces (see other ITSs noted above). No other platforms have the problem reported here, so we must consider it a bug in FreeBSD that munmapping a mutex doesn't automatically perform all of the necessary libc cleanup itself.
Hi, thanks for the headsup. Yes, it appears that the underlying issue was fixed earlier this year ( https://bugs.freebsd.org/269277 ).
Could you please revert just the older workaround portion, e.g.:
``` diff --git a/libraries/liblmdb/mdb.c b/libraries/liblmdb/mdb.c index 6e4c4d0..965ba2a 100644 --- a/libraries/liblmdb/mdb.c +++ b/libraries/liblmdb/mdb.c @@ -5824,17 +5824,6 @@ mdb_env_close0(MDB_env *env, int excl) if (excl > 0) semctl(env->me_rmutex->semid, 0, IPC_RMID); } -#elif defined(MDB_ROBUST_SUPPORTED) - /* If we have the filelock: If we are the - * only remaining user, clean up robust - * mutexes. - */ - if (excl == 0) - mdb_env_excl_lock(env, &excl); - if (excl > 0) { - pthread_mutex_destroy(env->me_txns->mti_rmutex); - pthread_mutex_destroy(env->me_txns->mti_wmutex); - } #endif munmap((void *)env->me_txns, (env->me_maxreaders-1)*sizeof(MDB_reader)+sizeof(MDB_txninfo)); } ```
and leave the rest (enabling robust mutexes on FreeBSD) as-is?
https://bugs.openldap.org/show_bug.cgi?id=9278
Howard Chu hyc@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |INVALID URL| |https://bugs.freebsd.org/26 | |9277 Status|CONFIRMED |RESOLVED
--- Comment #15 from Howard Chu hyc@openldap.org --- Partially reverted. 3dde6c46e6c55458eadaf7f81492c822414be2c7
https://bugs.openldap.org/show_bug.cgi?id=9278
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |VERIFIED
-
openldap-its＠openldap.org