revision
0001-Ticket-48311-nunc-stans-Attempt-to-release-connectio.patch

Thanks, Mark!!

9c84b93..97946bd master -> master
commit 97946bd
Author: Mark Reynolds mreynolds@redhat.com
Date: Fri Oct 23 15:17:44 2015 -0400

f27c164..a8d30b3 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit a8d30b3

Wrap calls with if statements to validate returns when we do not use PR_ASSERT
0001-Ticket-48311-nunc-stans-Attempt-to-release-connectio.2.patch

Hi,

I reviewed this, and I wondered why we would do something like:

connection_acquire_nolock(conn); /* event framework now has a reference */

But we wouldn't check the return. I've added a patch that checks these returns, but I want you to see if it's relevant or if there is a better way to handle this.

Thanks!

Replying to [comment:7 firstyear]:

Hi,

I reviewed this, and I wondered why we would do something like:

connection_acquire_nolock(conn); /* event framework now has a reference */

But we wouldn't check the return. I've added a patch that checks these returns, but I want you to see if it's relevant or if there is a better way to handle this.

Thanks!

This looks good, but your indentation is off. Looks like you used spaces instead of tabs. While we want to use spaces where possible, the surrounding code is using tabs, so it should match the existing code.

If checks and with proper tabs not soft tabs.
0001-Ticket-48311-nunc-stans-Attempt-to-release-connectio.3.patch

Okay, just wanting to confirm in all three cases that a straight return is an appropriate action to take. Should we be logging the error, or doing something else as well?

Replying to [comment:9 firstyear]:

Okay, just wanting to confirm in all three cases that a straight return is an appropriate action to take. Should we be logging the error, or doing something else as well?

Logging would be nice. If these conditions are hit it would be a serious issue.

A side note: without William's patch [1], we are having these warnings.
{{{
In file included from ldap/servers/slapd/slap.h:125:0,
from ldap/servers/slapd/daemon.c:48:
ldap/servers/slapd/daemon.c: In function 'ns_handle_closure':
../nunc-stans-0.1.7/include/nunc-stans/nunc-stans.h:305:37: warning: value computed is not used [-Wunused-value]
#define NS_JOB_IS_THREAD(eee) ((eee)&NS_JOB_THREAD)
^
ldap/servers/slapd/daemon.c:1839:2: note: in expansion of macro 'NS_JOB_IS_THREAD'
NS_JOB_IS_THREAD(ns_job_get_type(job));
^
ldap/servers/slapd/daemon.c: In function 'ns_handle_pr_read_ready':
../nunc-stans-0.1.7/include/nunc-stans/nunc-stans.h:305:37: warning: value computed is not used [-Wunused-value]
#define NS_JOB_IS_THREAD(eee) ((eee)&NS_JOB_THREAD)
^
ldap/servers/slapd/daemon.c:1922:2: note: in expansion of macro 'NS_JOB_IS_THREAD'
NS_JOB_IS_THREAD(ns_job_get_type(job));
^
}}}

[1] https://fedorahosted.org/389/attachment/ticket/48311/0001-Ticket-48311-nunc-stans-Attempt-to-release-connectio.3.patch

Add error logging for thread handlers.
0001-Ticket-48311-nunc-stans-Attempt-to-release-connectio.4.patch

Change to DEBUG_ANY
0001-Ticket-48311-nunc-stans-Attempt-to-release-connectio.5.patch

Attachment 0001-Ticket-48311-nunc-stans-Attempt-to-release-connectio.5.patch​ added
Change to DEBUG_ANY

Acked. Thanks, William!

commit 49aaf98

Pushed to master.

Pushed to 389-ds-base-1.3.4, as well:
d192435..b039876 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit b039876

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 issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1642

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.