Ticket 5103 - Remove unsafe use of pthread_cancel with PTHREAD_CANCEL_ASYNCHRONOUS
Summary: Remove unsafe use of pthread_cancel with PTHREAD_CANCEL_ASYNCHRONOUS
Status: RESOLVED FIXED
Alias: None
Product: Slurm
Classification: Unclassified
Component: Other (show other tickets)
Version: 17.11.5
Hardware: Linux Linux
: --- 3 - Medium Impact
Assignee: Tim Wickberg
QA Contact:
URL:
: 4690 4733 5111 5121 5177 5320 5545 (view as ticket list)
Depends on:
Blocks:
 
Reported: 2018-04-26 10:55 MDT by Tim Wickberg
Modified: 2023-03-02 09:02 MST (History)
19 users (show)

See Also:
Site: SchedMD
Alineos Sites: ---
Atos/Eviden Sites: ---
Confidential Site: ---
Coreweave sites: ---
Cray Sites: ---
DS9 clusters: ---
HPCnow Sites: ---
HPE Sites: ---
IBM Sites: ---
NOAA SIte: ---
OCF Sites: ---
Recursion Pharma Sites: ---
SFW Sites: ---
SNIC sites: ---
Linux Distro: ---
Machine Name:
CLE Version:
Version Fixed: 17.11.6 18.08.0-pre2
Target Release: ---
DevPrio: ---
Emory-Cloud Sites: ---

Attachments
change _watch_tasks to avoid use of pthread_cancel() (6.49 KB, patch)
2018-04-27 23:20 MDT, Tim Wickberg 		Details | Diff
mpi/pmix log entries (87.24 KB, image/tiff)
2018-04-28 14:52 MDT, Brian F Gilmer 		Details
Show Obsolete (1) Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this ticket.
Description Tim Wickberg 2018-04-26 10:55:33 MDT 
There are a number of locations where, to speed up process shutdown, threads have been marked as PTHREAD_CANCEL_ASYNCHRONOUS.

However, this is only safe when using async-cancel-safe functions (of which there are a total of three in libc). Use of malloc() can lead to deadlock.

Even with the thread set as PTHREAD_CANCEL_DEFERRED, use of any libc functions marked as cancellation points (see pthreads(7)) within a pthread_mutex_lock will mean that lock is not released, and can lead to deadlock.

Currently, this is leading to issues with the extern slurmstepd. Other locations appear to be significantly less likely, at least in part as the slurmstepd processes are started and stopped orders of magnitude more frequently than slurmctld/slurmdbd, but should be addressed as well.
Comment 1 Tim Wickberg 2018-04-26 11:01:07 MDT 
*** Ticket 4733 has been marked as a duplicate of this ticket. ***
Comment 2 Tim Wickberg 2018-04-26 11:02:53 MDT 
*** Ticket 4810 has been marked as a duplicate of this ticket. ***
Comment 3 Tim Wickberg 2018-04-26 11:05:26 MDT 
*** Ticket 4690 has been marked as a duplicate of this ticket. ***
Comment 5 Tim Wickberg 2018-04-27 23:20:10 MDT 
Created attachment 6718 [details]
change _watch_tasks to avoid use of pthread_cancel()

Hi folks -

This is a first pass at removing what we currently believe are the race condition triggering the extern (or normal user) step deadlocks.

If you're able to test this I would certainly appreciate any feedback, positive or negative.

There are still similar issues in the IPMI and PMIx plugins, and we will be working to clear those up at some point, although those seem to be much rarer in practice.

- Tim
Comment 8 Brian F Gilmer 2018-04-28 14:48:09 MDT 
Hi Tim,

We checked the logs today and we saw 2500 instances of slurmstepd spinning in mpi/pmix code.  We did not see any of the other slurmstepd deadlock.  I had the site save off a backtrace and excerpts from the slurmd log.  When the site has processed them I will attach them to the bug.
Comment 9 Brian F Gilmer 2018-04-28 14:52:29 MDT 
Created attachment 6722 [details]
mpi/pmix log entries

Log entries for the slurmstepd in mpi/pmix
Comment 16 Tim Wickberg 2018-04-30 16:40:21 MDT 
Just an update - 

We did land a series of commits similar to attachment 6718 [details]  that should address this in most instances, and will be in 17.11.6 when released. We do still have some work left on the PMIx plugin; if you don't use that plugin we believe those patches are sufficient to prevent this from happening.

We'll attach the PMIx cleanup here as well when available. If you're not in a rush, I would suggest waiting until 17.11.6 is released (should be within a week). Or you can use that patch in the meantime, or cherry-pick commits 
1675ada0a, a7c8964e, and 3be9e1ee0 from git.

- Tim
Comment 17 Christopher Samuel 2018-04-30 21:07:29 MDT 
On 1/5/18 8:40 am, bugs@schedmd.com wrote:

> Just an update -

Thanks Tim, we'll wait for 17.11.6 to land to test, we only see this
currently when one of my colleagues runs a series of test jobs (so far,
fingers crossed!).

All the best,
Chris
Comment 18 Tim Wickberg 2018-05-01 17:33:27 MDT 
*** Ticket 5121 has been marked as a duplicate of this ticket. ***
Comment 20 Tim Wickberg 2018-05-03 00:12:13 MDT 
I ended up splitting the PMIx portion of this off on commit e5f03971b / bug 5119, and that's available if you'd like to apply that. We expect to release 17.11.6 soon with all of these fixes rolled in shortly, and I am going ahead and tagging this as resolved.

If you're still noticing issues after that, please file a separate issue to discuss that further.

Thanks for you patience on all of this, the underlying bug proved quite difficult to isolate which unfortunately delayed this quite a bit longer than we'd like.

- Tim
Comment 21 Dominik Bartkiewicz 2018-05-03 13:18:31 MDT 
*** Ticket 5111 has been marked as a duplicate of this ticket. ***
Comment 22 Marshall Garey 2018-05-16 09:23:30 MDT 
*** Ticket 5177 has been marked as a duplicate of this ticket. ***
Comment 23 Alejandro Sanchez 2018-07-02 04:32:07 MDT 
*** Ticket 5320 has been marked as a duplicate of this ticket. ***
Comment 25 Marshall Garey 2018-09-04 11:17:24 MDT 
*** Ticket 5545 has been marked as a duplicate of this ticket. ***
Comment 26 Regine Gaudin 2019-07-29 03:38:34 MDT 
Hi

CEA is falling in this bug encountred in 17.11.6 and fixed in 17.11.7 
+ -- Fix slurmstepd deadlock in stepd cleanup caused by race condition in
+    the jobacct_gather fini() interfaces introduced in 17.11.6.

Would it be possible to have access to the patch ?
Thanks
Regine Gaudin
Comment 27 Naveed Near-Ansari 2019-07-29 03:38:51 MDT 
I will we be out of the officeUntil August 5th.  If you need assistance, please email help-hpc@caltech.edu.  There will be others around that can deal with anything that may come up.
Comment 28 Christopher Samuel 2019-07-29 09:48:02 MDT 
(In reply to Regine Gaudin from comment #26)

> CEA is falling in this bug encountred in 17.11.6 and fixed in 17.11.7 
> + -- Fix slurmstepd deadlock in stepd cleanup caused by race condition in
> +    the jobacct_gather fini() interfaces introduced in 17.11.6.
> 
> Would it be possible to have access to the patch ?

Answering as someone who hit this bug when I was in Australia and got your reply to this resolved bugzilla entry.

17.11.9 was the last version of 17.11 released (that branch is now obsolete now 19.05 is out):

https://www.schedmd.com/archives.php

All the best,
Chris
Comment 29 Jason Booth 2019-07-29 10:19:29 MDT 
Hi Regine and Chris -

 The last version of 17.11 was 17.11.13-2 and as Chris has mentioned the 17.11 release is no longer supported. 

 In regards to your request for patches, there were multiple issues and a series of commits that fixed the issue, so it would be advisable to upgrade at this point in time.

-Jason