389-ds-base

#50268 Ticket 49873: (cont) Contention on virtual attribute lookup
Closed by spichugi. Opened by tbordaz.
tbordaz/389-ds-base ticket_49873_cont  into  master

Download 50268.patch

Bug Description:
The previous fix was incomplete.
First it created the thread private counter before the fork.
The deamon process was not inheriting it.

Second there is a possiblity that an callback of an internal search
tries to update the map. (cos thread monitoring cos definition)
In such case the RW lock was first acquired in read at the top level
of the internal search, then later the callback try to acquire it in write.
this created a deadlock

Fix Description:
The fix consists to create the thread private counter after the deamon creation.
In adding, when acquiring the lock in write, if the lock was already acquired
at the top level (in read), it release the lock and reset the counter. Then acquires
the lock in write.
In the opposite when releasing the lock in read, if the lock was not already acquired
it assumes it was acquired in write and do nothing

https://pagure.io/389-ds-base/issue/49873

Reviewed by: ?

Platforms tested: F30

Flag Day: no

Doc impact: no

int32_t

There should be no case where there isn't a thread_private_global_vattr_lock though, so why do we check this condition? If we are missing this ,then this could hit a self deadlock case?

Certainly, this is a bug as I think of it. The condition is checking "do we have a thread private value present", not "what is the content of the thread private value. So I think perhaps this could be the cause of some confusion?

@tbordaz, you are still not using the integer pointers correctly. There are a bunch of compiler warnings like:

../389-ds-base/ldap/servers/slapd/vattr.c: In function ‘vattr_rdlock’:
../389-ds-base/ldap/servers/slapd/vattr.c:150:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
         int nb_acquire = (int) PR_GetThreadPrivate(thread_private_global_vattr_lock);

Please see this comment on how to fix it:

https://pagure.io/389-ds-base/issue/49873#comment-557494

Thanks,
Mark

At the moment the private index is defined but if it was not (decision to disable this mechanism or a bug), it will fallback to previous behavior.

@tbordaz I think I'd rather see PR_ASSERT(private index) and have the "old behaviour" in #ifdef instead of doing an actual if check every time if the intent is to "feature gate" and roll back.

rebased onto 1a9d414522c65b21baa0746d2e215fdc7a37ddfc

This version appears to have fixed the deadlock I was encountering, ack

rebased onto 6d0ba294b30931933401b98768843a36fbe76632

Pull-Request has been merged by tbordaz

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3327

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.