PR#49869: Ticket 49848 - Nunc-stans event thread can be hanging shortly when a job is (re)armed - 389-ds-base

389-ds-base

#49869 Ticket 49848 - Nunc-stans event thread can be hanging shortly when a job is (re)armed

Closed 3 years ago by spichugi. Opened 5 years ago by tbordaz.

tbordaz/389-ds-base ticket-49848 into master

Ticket 49848 - Nunc-stans event thread can be hanging shortly when a job is (re)armed

Thierry Bordaz • 5 years ago

5e39c59

src/nunc-stans/ns/ns_thrpool.c

file modified

+12 -2

		`@@ -260,6 +260,15 @@`

		`/* I think we need to check about is_shutdown here? */`

		`+ /*`
		`+ * weither it will wakeup a NS worker thread (work_q) or the NS event thread (event_q)`
		`+ * This thread will acquire the job->monitor.`
		`+ * To prevent the thread to wait just release the lock before waking it`
		`+ *`
		`+ * This is specifically important for the event thread`
		`+ */`
		`+ pthread_mutex_unlock(&(job->monitor));`
		`+`
		`if (NS_JOB_IS_IO(job->job_type) \|\| NS_JOB_IS_TIMER(job->job_type) \|\| NS_JOB_IS_SIGNAL(job->job_type)) {`
		`event_q_notify(job);`
		`} else {`
		`@@ -267,7 +276,6 @@`
		`/* Prevents an un-necessary queue / dequeue to the event_q */`
		`work_q_notify(job);`
		`}`
		`- pthread_mutex_unlock(&(job->monitor));`
		`}`

		`static void`
		`@@ -1027,6 +1035,7 @@`
		`if (!_job) {`
		`return NS_ALLOCATION_FAILURE;`
		`}`
		`+ pthread_mutex_lock(&(_job->monitor));`
		`/* fill in a pointer to the job for the caller if requested */`
		`if (job) {`
		`*job = _job;`
		`@@ -1036,6 +1045,7 @@`
		`ns_log(LOG_DEBUG, "ns_add_job %x state %d moving to NS_JOB_ARMED\n", _job, (_job)->state);`
		`#endif`
		`_job->state = NS_JOB_NEEDS_ARM;`
		`+ pthread_mutex_unlock(&(_job->monitor));`
		`internal_ns_job_rearm(_job);`

		`return NS_SUCCESS;`
		`@@ -1218,8 +1228,8 @@`
		`ns_log(LOG_DEBUG, "ns_rearm_job %x state %d moving to NS_JOB_NEEDS_ARM\n", job, job->state);`
		`#endif`
		`job->state = NS_JOB_NEEDS_ARM;`
		`- internal_ns_job_rearm(job);`
		`pthread_mutex_unlock(&(job->monitor));`
		`+ internal_ns_job_rearm(job);`
		`return NS_SUCCESS;`
		`} else if (!NS_JOB_IS_PERSIST(job->job_type) && job->state == NS_JOB_RUNNING) {`
		`/* For this to be called, and NS_JOB_RUNNING, we must be the callback thread! */`

tbordaz commented 5 years ago

Bug Description:
When a nunc-stans job is rearmed, event_q_notify enqueue the job and notify the event thread.

The first thing event thread does is to lock the job.
But until internal_ns_job_rearm release the lock, event thread is hanging.

This is also the case of a worker thread (work_q_notify) but has less negative impact

Fix Description:
Release the lock of the job before calling event_q_notify/work_q_notify

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

Reviewed by: ?

Platforms tested: F27

Flag Day: no

Doc impact: no

tbordaz commented 5 years ago

This PR was a tentative to improve the situation on nunc-stans.
@firstyear create a new PR https://pagure.io/389-ds-base/pull-request/49636 that is a larger change and that is the priority. We can not say if https://pagure.io/389-ds-base/pull-request/49869 still applies. So just cancel it

Pull-Request has been closed by tbordaz

5 years ago

spichugi commented 3 years ago

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

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

3 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 1

+12 -2

file changed

src/nunc-stans/ns/ns_thrpool.c

389-ds-base

Source Code

#49869 Ticket 49848 - Nunc-stans event thread can be hanging shortly when a job is (re)armed Closed 3 years ago by spichugi. Opened 5 years ago by tbordaz. tbordaz/389-ds-base ticket-49848 into master

Metadata

Changes Summary 1

#49869 Ticket 49848 - Nunc-stans event thread can be hanging shortly when a job is (re)armed

Closed 3 years ago by spichugi. Opened 5 years ago by tbordaz.

tbordaz/389-ds-base ticket-49848 into master