Issue 6745 - HEAD emfile race condition -> slapd stopping listening?
Summary: HEAD emfile race condition -> slapd stopping listening?
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: 2.5.0
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-13 12:49 UTC by Hallvard Furuseth
Modified: 2020-10-14 21:01 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Hallvard Furuseth 2010-12-13 12:49:00 UTC
Full_Name: Hallvard B Furuseth
Version: HEAD
OS: 
URL: 
Submission from: (NULL) (129.240.6.233)
Submitted by: hallvard


In HEAD slapd/daemon.c, thanks to HEAD's multiple listener support:

  slap_listener() protects emfile with slap_daemon[0 ].sd_mutex,
  but slapd_remove() protects it  with slap_daemon[id].sd_mutex.

Maybe to compensate, slapd_remove() has code which checks if emfile
is too big, but nothing checks if it is too small - which looks like
slapd might never start listening again.

Simplest fix: add this to slapd_remove():

   if (id) ldap_pvt_thread_mutex_<lock,unlock>( &slap_daemon[0].sd_mutex );

slap_daemon[0].sd_mutex looks quite contended that way, though.
Maybe a separate emfile_mutex is better.
Comment 1 Howard Chu 2010-12-24 00:15:10 UTC
h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD
> OS:
> URL:
> Submission from: (NULL) (129.240.6.233)
> Submitted by: hallvard
>
>
> In HEAD slapd/daemon.c, thanks to HEAD's multiple listener support:
>
>    slap_listener() protects emfile with slap_daemon[0 ].sd_mutex,
>    but slapd_remove() protects it  with slap_daemon[id].sd_mutex.
>
> Maybe to compensate, slapd_remove() has code which checks if emfile
> is too big, but nothing checks if it is too small - which looks like
> slapd might never start listening again.
>
> Simplest fix: add this to slapd_remove():
>
>     if (id) ldap_pvt_thread_mutex_<lock,unlock>(&slap_daemon[0].sd_mutex );
>
> slap_daemon[0].sd_mutex looks quite contended that way, though.
> Maybe a separate emfile_mutex is better.

Alternatively, just skip the check entirely when id != 0. Which would mean 
that only closing a session on the first listener would ever trigger the other 
listeners to be unmuted. Doesn't seem so terrible; if slapd is actually out of 
descriptors one or two connections either way won't make a huge difference in 
service.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 2 Howard Chu 2010-12-24 01:56:10 UTC
Howard Chu wrote:
> h.b.furuseth@usit.uio.no wrote:
>> Full_Name: Hallvard B Furuseth
>> Version: HEAD
>> OS:
>> URL:
>> Submission from: (NULL) (129.240.6.233)
>> Submitted by: hallvard
>>
>>
>> In HEAD slapd/daemon.c, thanks to HEAD's multiple listener support:
>>
>>     slap_listener() protects emfile with slap_daemon[0 ].sd_mutex,
>>     but slapd_remove() protects it  with slap_daemon[id].sd_mutex.
>>
>> Maybe to compensate, slapd_remove() has code which checks if emfile
>> is too big, but nothing checks if it is too small - which looks like
>> slapd might never start listening again.

On 2nd thought - I don't see how the outcome you describe can occur. If emfile 
is non-zero, the list of listeners will be checked for a listener with 
non-zero mute status. If any are found, one is guaranteed to be unmuted. If 
none are found, emfile will be zeroed.

In no case will slapd fail to unmute a listener.
-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 3 Hallvard Furuseth 2010-12-28 14:35:48 UTC
Howard Chu wrote:
>>h.b.furuseth@usit.uio.no wrote:
>>> Maybe to compensate, slapd_remove() has code which checks if emfile
>>> is too big, but nothing checks if it is too small - which looks like
>>> slapd might never start listening again.
> 
> On 2nd thought - I don't see how the outcome you describe can occur. If emfile
> is non-zero, the list of listeners will be checked for a listener with 
> non-zero mute status. If any are found, one is guaranteed to be unmuted. 

But if emfile becomes too small, it reaches zero with a listener still
muted - and with emfile = zero, the nothing is unmuted.

-- 
Hallvard

Comment 4 OpenLDAP project 2017-04-07 23:45:58 UTC
Needs fixing for 2.5?
Comment 5 Quanah Gibson-Mount 2017-04-07 23:45:58 UTC
changed notes
moved from Incoming to Software Bugs