Re: (ITS#5814) concurrent access to connections

[ando@sys-net.it]

hyc@symas.com wrote:
> ando@sys-net.it wrote:
>> Hallvard B Furuseth wrote:
>>> test036-meta-concurrency just crashed with fresh HEAD code.
>>> I suppose that's the same as this ITS.
>> Looks like.  However, now I thought my FIXME were little more than a
>> precaution.  In fact, right now the only places where c_struct_state can
>> be modified are protected by a mutex on connections_mutex; this should
>> prevent changing the state of a connection during a connection_next()
>> call.  Do you see a thread waiting on
>> ldap_pvt_thread_mutex_lock(&connections_mutex ); at connection.c:427 or
>> connection.c:514?  If that's the case, we probably need to move
>> c->c_conn_state = SLAP_C_INACTIVE; inside the mutex in order to be able
>> to safely assert in connection_next().  Is it worth?
> 
> That should not be necessary. c_struct_state is supposed to be protected by 
> connections_mutex, while c_conn_state is supposed to be protected by 
> c->c_mutex. As already described in the comments at the top of the file.

What seesm to happen is that c_conn_state is changed, then 
connections_mutex prevents from changing c_struct_state.  As a 
consequence, connection_next() treats a connection structure based on 
its c_struct_state, which is now correctly protected, but then asserts 
on its c_conn_state, which has already been modified.  So we are 
asserting on the state of a connection that is being modified, and is by 
design in a temporarily inconsistent state.  Now things should be safe, 
though.

p.


Ing. Pierangelo Masarati
OpenLDAP Core Team

SysNet s.r.l.
via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
-----------------------------------
Office:  +39 02 23998309
Mobile:  +39 333 4963172
Fax:     +39 0382 476497
Email:   ando@sys-net.it
-----------------------------------