389-ds-base

#50492 Ticket 50459 - Correct issue with allocation state
Closed by spichugi. Opened by firstyear.
firstyear/389-ds-base 50459-c_state-invalid  into  master

Download 50492.patch

Bug Description: While adding the connection state, due to
a misunderstanding on my part, it was possible that a connection
was more likely to fail to allocate causing the server to exit(1)
incorrectly.

Fix Description: Fix the state handler to correctly account for
connection structure reuse.

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

Author: William Brown william@blackhats.net.au

Review by: ???

@firstyear, I agree with the fix but disagree with to make it on top of https://pagure.io/389-ds-base/pull-request/50485

50459 and PR 50485 are to change c_mutex into pthread_mutex

The current PR https://pagure.io/389-ds-base/pull-request/50492# is related to #50489 that I opened yesterday. This is an independent problem from #50459. It is about make connection_table_get_connection thread safe.

I link https://pagure.io/389-ds-base/pull-request/50492# with https://pagure.io/389-ds-base/issue/50489.
As I said the fix looks good. Please rebase it on master not top of pending #50459 .

@tbordaz 50459 was merged to master? So this was based off from master actually, not from 50459. So maybe I misunderstand your concern here, because this fix is directly related to the code I merged to master, and it has a potential issue in it.

@firstyear, Sorry my mistake. I missed that you already merged https://pagure.io/389-ds-base/pull-request/50485 (#50459) into master.

We have #50459 that is to move c_mutex from NSPR to pthread_lock. It is fixed with https://pagure.io/389-ds-base/pull-request/50485.

I do not like the idea to have a second PR for #50459. This second PR is to make connection_table_get_connection thread safe that is #50489. #50489 should not be closed as duplicate as #50459 as it is a different problem. Also it is error prone while if we need to frontport or backout a fix.

The patch is valid and you have my ACK. Could you update this PR so that is only linked to #50489.

This patch isn't to address a nunc-stans issue like in #50489 - it's directly a bug as a result of the work in #50459. That's why I linked it as such.

You have acked though, so I'll merge, because it's going to be annoyig if people hit this in development.

rebased onto d5b23dcb89a75f437c410c6b91e0444fe1dccb0a

Pull-Request has been merged by firstyear

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/3549

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.