openldap-bugs

openldap-bugs@openldap.org

1 participants
20961 discussions

Re: (ITS#7377) Poor libmdb error handling
by hyc＠symas.com 01 Sep '12

01 Sep '12

h.b.furuseth(a)usit.uio.no wrote: > 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?) In general I prefer that functions have only one exit point, so multiple gotos is preferable. > 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(). That's really not interesting to me. No other module has any business opening and reading our lock file. We haven't bothered with this for alock either, and it hasn't been a problem. Change of pid... not worth bothering. Just add a note to the docs. > 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. Agreed. I had a patch here that does just that, seems it got lost in some other edits. > OTOH > some strategically placed getpid()s can then catch the user error > of having forked, and set e.g. MDB_FATAL_ERROR. Not really interesting. > The fork() restriction needs to be documented, as does "don't open > or close the same database twice in the same process". Go ahead. > 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. Called twice on the same env handle? Again, not interesting. As a rule, we should of course check for failure return codes from normal operation. We should not add extensive checks for programmer error/misuse of API. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#7377) Poor libmdb error handling
by openldap-its＠OpenLDAP.org 01 Sep '12

01 Sep '12

I messed up last reply headers somehow. Trying again. I wrote: > 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. That is, "cannot happen" when there is no user error. > The fork() restriction needs to be documented, as does "don't > open or close the same database twice in the same process". Correction, "don't have the same database open twice at the same time in the same process".

1 0

Re: (ITS#7377) Poor libmdb error handling
by openldap-its＠OpenLDAP.org 01 Sep '12

01 Sep '12

I wrote: > 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. That is, "cannot happen" when there is no user error. > The fork() restriction needs to be documented, as does "don't > open or close the same database twice in the same process". Correction, "don't have the same database open twice at the same time in the same process".

1 0

(ITS#7377) Poor libmdb error handling
by h.b.furuseth＠usit.uio.no 01 Sep '12

01 Sep '12

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.

1 0

Re: (ITS#7375) slapo-dynlist is very slow
by hyc＠symas.com 31 Aug '12

31 Aug '12

rouazana(a)linagora.com wrote: > Note that using "sortvals uniqueMember" configuration allows dynlist to > work in a correct time (about 3s for a base search on the group). > > So I don't know I you want this issue to be fixed or not. Closing this ITS. > > Regards, > Raphaël Ouazana. > > > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#7375) slapo-dynlist is very slow
by rouazana＠linagora.com 31 Aug '12

31 Aug '12

Note that using "sortvals uniqueMember" configuration allows dynlist to work in a correct time (about 3s for a base search on the group). So I don't know I you want this issue to be fixed or not. Regards, Raphaël Ouazana.

1 0

(ITS#7249) slapd segfault with memberof overlay on frontend db
by c.phan＠active-system.com 31 Aug '12

31 Aug '12

Full_Name: Chinh Phan Version: 2.4.32 OS: Fedora 16 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (84.96.74.38) Hello, I'm still having a segfault error even with the fix suggested by Howard Chu (http://www.openldap.org/lists/openldap-bugs/201204/msg00171.html): slapd[2812]: segfault at 7f8d0b7fffd8 ip 00000000004377b0 sp 00007f8d0b7fffe0 error 6 in slapd[400000+1c6000] Does anyone know if this issue has been addressed or is planned to be fixed in the next release? Appreciate your feedback. chinh

1 0

(ITS#7375) slapo-dynlist is very slow
by raphael.ouazana＠linagora.com 31 Aug '12

31 Aug '12

Full_Name: Raphael Ouazana Version: 2.4.31 OS: Linux URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (88.173.78.196) Hi, I'm using dynlist to generate a dynamic group with ~50k users. The LDAP request written in the memberURL attribute takes about 8 seconds to execute. But a base search request on the group takes more than 2 minutes to execute. So I think dynlist should be optimized. Regards, Raphaël Ouazana.

1 0

(ITS#7374) [PATCH] MozNSS: better file name matching for hashed CA certificate directory
by jvcelak＠redhat.com 29 Aug '12

29 Aug '12

Full_Name: Jan Vcelak Version: git master OS: Linux URL: ftp://ftp.openldap.org/incoming/jvcelak-20120829-moznss-beter-file-name-mat… Submission from: (NULL) (209.132.186.34) This change concerns Mozilla NSS crypto backend: CA certificate files in OpenSSL compatible CACERTDIR were loaded if the file extension was '.0'. However the file name should be 8 letters long certificate hash of the certificate subject name, followed by a numeric suffix which is used to differentiate between two certificates with the same subject name. With this patch, certificate file names are matched correctly (using regular expressions). The attached file is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Red Hat. Red Hat has not assigned rights and/or interest in this work to any party. I, Jan Vcelak am authorized by Red Hat, my employer, to release this work under the following terms. Red Hat hereby place 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.

1 0

Re: (ITS#7363) libmdb should use POSIX semaphores on non-apple BSD systems too.
by cmikk＠qwest.net 29 Aug '12

29 Aug '12

On Wed, Aug 29, 2012 at 08:07:31AM +0200, Hallvard Breien Furuseth wrote: > We can #ifndef PTHREAD_PROCESS_SHARED instead of BSD, unless someone > #define it without supporting it. Not for MDB_FDATASYNC though. The *BSD systems define PTHREAD_PROCESS_SHARED but do not implement sharable pthread mutexes. The flag name is part of the API, but implementation of the behavior it requests is optional. -- Chris Mikkelson | Quidquid latine dictum sit, altum viditur cmikk(a)qwest.net |

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs