Releases, locking and ldb

[Andrew Bartlett]

On Tue, 2017-06-27 at 08:24 +0200, Stefan Metzmacher via samba-
technical wrote:
> Am 27.06.2017 um 04:36 schrieb Andrew Bartlett via samba-technical:
> > On Tue, 2017-06-27 at 08:07 +1200, Andrew Bartlett via samba-technical
> > wrote:
> > > On Mon, 2017-06-26 at 18:11 +0200, Stefan Metzmacher wrote:
> > > > 
> > > > Would sssd break with these changes?
> > > > 
> > > > Do we have other known external users, beside openchange?
> > > 
> > > I'm not sure.  I'll check with apt reverse depends when I get to the
> > > office. 
> > 
> > For Debian:
> > 
> > libldb1
> >   Reverse Depends: ldb-tools (>= 2:1.1.27-1+b1)
> >   Reverse Depends: libldb-dev (= 2:1.1.27-1+b1)
> >   Reverse Depends: python-ldb (= 2:1.1.27-1+b1)
> >   Reverse Depends: python-samba (>= 2:4.5.8+dfsg-2)
> >   Reverse Depends: python-sss (>= 1.15.0-3)
> >   Reverse Depends: python3-sss (>= 1.15.0-3)
> >   Reverse Depends: samba (>= 2:4.5.8+dfsg-2)
> >   Reverse Depends: samba-dsdb-modules (>> 2:4.5.8+dfsg-2)
> >   Reverse Depends: samba-libs (>= 2:4.5.8+dfsg-2)
> >   Reverse Depends: samba-testsuite (>= 2:4.5.8+dfsg-2)
> >   Reverse Depends: sssd-ad-common (>= 1.15.0-3)
> >   Reverse Depends: sssd-common (>= 1.15.0-3)
> >   Reverse Depends: sssd-dbus (>= 1.15.0-3)
> >   Reverse Depends: sssd-proxy (>= 1.15.0-3)
> >   Reverse Depends: sssd-tools (>= 1.15.0-3)
> > 
> > That limits the fallout pretty well it seems.
> 
> The good thing is that due to LDB_MODULE_CHECK_VERSION()
> Samba and sssd are only runtime compatible with the version
> they're build against.
> 
> Would "ldb:tdb: Ensure we correctly decrement ltdb->read_lock_count"
> affect any old caller in any bad way?

I presume you mean some other hypothetical application beyond Samba and
SSSD?

The primary change is that during the ldb callbacks, the DB is locked,
rather than unlocked.  It is no longer possible to start a transaction
(write operation) during an ldb callback.  Instead, as good practice
would already dictate, you must start the transaction before the
search. 

> And what about "tdb: Remove locking from tdb_traverse_read()"?

I'm not sure about this one.  As we can't have one without the other I
don't know (and haven't studied) exactly what else it might impact. 

I don't think this without the ldb patches is actively harmful however.

> Whould something like this on the samba_dsdb module
> in 4.6 work around the problems (and keep it only as broken as it
> already is):
> - always call ldb_handle_use_global_event_context()

I don't think this case is worth worrying about.  It is rootdse
modified over LDAP in the single process model (only). 

> - overwrite read_lock/read_unlock() to be a no-op

No, this gets us to the point I was at during SambaXP, with random
LDB_ERR_BUSY / 'failed to upgrade hash locks', as we won't have a
global lock order.

I don't think it makes any difference to actually to have it pass the
read_lock()/read_unlock() down to sam.ldb.

I personally think we should just declare the incomparability, ideally
at build time, and move on.  (I tried to create an #ifdef hack to
detect this at build time, but wasn't able to build a reliable one
yet). 

I realise that everything that can happen will happen, but still: how
likely are they to be mixed in the real world?

Sorry,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba