From 91504a0e0d170b284e460c665235eaa2a041322d Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Wed, 8 May 2013 16:02:59 -0700 Subject: [PATCH] [release-branch.go1.1] runtime: fix crash in select MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit ««« CL 9311043 / 53bc96b4c0c7 runtime: fix crash in select runtime.park() can access freed select descriptor due to a racing free in another thread. See the comment for details. Slightly modified version of dvyukov's CL 9259045. No test yet. Before this CL, the test described in issue 5422 would fail about every 40 times for me. With this CL, I ran the test 5900 times with no failures. Fixes #5422. R=golang-dev, r CC=golang-dev https://golang.org/cl/9311043 »»» R=golang-dev, r CC=golang-dev https://golang.org/cl/9304044 --- src/pkg/runtime/chan.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/pkg/runtime/chan.c b/src/pkg/runtime/chan.c index 69b90bda56..fba36a4c34 100644 --- a/src/pkg/runtime/chan.c +++ b/src/pkg/runtime/chan.c @@ -809,16 +809,27 @@ sellock(Select *sel) static void selunlock(Select *sel) { - uint32 i; - Hchan *c, *c0; + int32 i, n, r; + Hchan *c; - c = nil; - for(i=sel->ncase; i-->0;) { - c0 = sel->lockorder[i]; - if(c0 && c0 != c) { - c = c0; - runtime·unlock(c); - } + // We must be very careful here to not touch sel after we have unlocked + // the last lock, because sel can be freed right after the last unlock. + // Consider the following situation. + // First M calls runtime·park() in runtime·selectgo() passing the sel. + // Once runtime·park() has unlocked the last lock, another M makes + // the G that calls select runnable again and schedules it for execution. + // When the G runs on another M, it locks all the locks and frees sel. + // Now if the first M touches sel, it will access freed memory. + n = (int32)sel->ncase; + r = 0; + // skip the default case + if(n>0 && sel->lockorder[0] == nil) + r = 1; + for(i = n-1; i >= r; i--) { + c = sel->lockorder[i]; + if(i>0 && sel->lockorder[i-1] == c) + continue; // will unlock it on the next iteration + runtime·unlock(c); } } -- 2.50.0