Skip to content

Commit dbab079

Browse files
committed
runtime: free Windows event handles after last lock is dropped
Calls to lock may need to use global members of mOS that also need to be cleaned up before the thread exits. Before this commit, these resources would leak. Moving them to be cleaned up in unminit, however, would race with gstack on unix. So this creates a new helper, mdestroy, to release resources that must be destroyed only after locks are no longer required. We also move highResTimer lifetime to the same semantics, since it doesn't help to constantly acquire and release the timer object during dropm. Updates #43720. Change-Id: Ib3f598f3fda1b2bbcb608099616fa4f85bc1c289 Reviewed-on: https://go-review.googlesource.com/c/go/+/284137 Run-TryBot: Jason A. Donenfeld <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Austin Clements <[email protected]> Trust: Alex Brainman <[email protected]> Trust: Jason A. Donenfeld <[email protected]>
1 parent 5a8fbb0 commit dbab079

12 files changed

+80
-12
lines changed

src/runtime/os3_solaris.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,11 @@ func unminit() {
227227
unminitSignals()
228228
}
229229

230+
// Called from exitm, but not from drop, to undo the effect of thread-owned
231+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
232+
func mdestroy(mp *m) {
233+
}
234+
230235
func sigtramp()
231236

232237
//go:nosplit

src/runtime/os_aix.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ func unminit() {
180180
unminitSignals()
181181
}
182182

183+
// Called from exitm, but not from drop, to undo the effect of thread-owned
184+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
185+
func mdestroy(mp *m) {
186+
}
187+
183188
// tstart is a function descriptor to _tstart defined in assembly.
184189
var tstart funcDescriptor
185190

src/runtime/os_darwin.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,11 @@ func unminit() {
325325
}
326326
}
327327

328+
// Called from exitm, but not from drop, to undo the effect of thread-owned
329+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
330+
func mdestroy(mp *m) {
331+
}
332+
328333
//go:nosplit
329334
func osyield() {
330335
usleep(1)

src/runtime/os_dragonfly.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,11 @@ func unminit() {
203203
unminitSignals()
204204
}
205205

206+
// Called from exitm, but not from drop, to undo the effect of thread-owned
207+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
208+
func mdestroy(mp *m) {
209+
}
210+
206211
func sigtramp()
207212

208213
type sigactiont struct {

src/runtime/os_freebsd.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,11 @@ func unminit() {
319319
unminitSignals()
320320
}
321321

322+
// Called from exitm, but not from drop, to undo the effect of thread-owned
323+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
324+
func mdestroy(mp *m) {
325+
}
326+
322327
func sigtramp()
323328

324329
type sigactiont struct {

src/runtime/os_js.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ func minit() {
8484
func unminit() {
8585
}
8686

87+
// Called from exitm, but not from drop, to undo the effect of thread-owned
88+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
89+
func mdestroy(mp *m) {
90+
}
91+
8792
func osinit() {
8893
ncpu = 1
8994
getg().m.procid = 2

src/runtime/os_linux.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,11 @@ func unminit() {
375375
unminitSignals()
376376
}
377377

378+
// Called from exitm, but not from drop, to undo the effect of thread-owned
379+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
380+
func mdestroy(mp *m) {
381+
}
382+
378383
//#ifdef GOARCH_386
379384
//#define sa_handler k_sa_handler
380385
//#endif

src/runtime/os_netbsd.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,11 @@ func unminit() {
290290
unminitSignals()
291291
}
292292

293+
// Called from exitm, but not from drop, to undo the effect of thread-owned
294+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
295+
func mdestroy(mp *m) {
296+
}
297+
293298
func sigtramp()
294299

295300
type sigactiont struct {

src/runtime/os_openbsd.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,11 @@ func unminit() {
257257
unminitSignals()
258258
}
259259

260+
// Called from exitm, but not from drop, to undo the effect of thread-owned
261+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
262+
func mdestroy(mp *m) {
263+
}
264+
260265
func sigtramp()
261266

262267
type sigactiont struct {

src/runtime/os_plan9.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,11 @@ func minit() {
213213
func unminit() {
214214
}
215215

216+
// Called from exitm, but not from drop, to undo the effect of thread-owned
217+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
218+
func mdestroy(mp *m) {
219+
}
220+
216221
var sysstat = []byte("/dev/sysstat\x00")
217222

218223
func getproccount() int32 {

src/runtime/os_windows.go

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -898,20 +898,18 @@ func minit() {
898898
throw("runtime.minit: duplicatehandle failed")
899899
}
900900

901+
mp := getg().m
902+
lock(&mp.threadLock)
903+
mp.thread = thandle
904+
901905
// Configure usleep timer, if possible.
902-
var timer uintptr
903-
if haveHighResTimer {
904-
timer = createHighResTimer()
905-
if timer == 0 {
906+
if mp.highResTimer == 0 && haveHighResTimer {
907+
mp.highResTimer = createHighResTimer()
908+
if mp.highResTimer == 0 {
906909
print("runtime: CreateWaitableTimerEx failed; errno=", getlasterror(), "\n")
907910
throw("CreateWaitableTimerEx when creating timer failed")
908911
}
909912
}
910-
911-
mp := getg().m
912-
lock(&mp.threadLock)
913-
mp.thread = thandle
914-
mp.highResTimer = timer
915913
unlock(&mp.threadLock)
916914

917915
// Query the true stack base from the OS. Currently we're
@@ -947,13 +945,29 @@ func minit() {
947945
func unminit() {
948946
mp := getg().m
949947
lock(&mp.threadLock)
950-
stdcall1(_CloseHandle, mp.thread)
951-
mp.thread = 0
948+
if mp.thread != 0 {
949+
stdcall1(_CloseHandle, mp.thread)
950+
mp.thread = 0
951+
}
952+
unlock(&mp.threadLock)
953+
}
954+
955+
// Called from exitm, but not from drop, to undo the effect of thread-owned
956+
// resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
957+
//go:nosplit
958+
func mdestroy(mp *m) {
952959
if mp.highResTimer != 0 {
953960
stdcall1(_CloseHandle, mp.highResTimer)
954961
mp.highResTimer = 0
955962
}
956-
unlock(&mp.threadLock)
963+
if mp.waitsema != 0 {
964+
stdcall1(_CloseHandle, mp.waitsema)
965+
mp.waitsema = 0
966+
}
967+
if mp.resumesema != 0 {
968+
stdcall1(_CloseHandle, mp.resumesema)
969+
mp.resumesema = 0
970+
}
957971
}
958972

959973
// Calling stdcall on os stack.

src/runtime/proc.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,6 +1407,10 @@ found:
14071407
}
14081408
}
14091409

1410+
// Destroy all allocated resources. After this is called, we may no
1411+
// longer take any locks.
1412+
mdestroy(m)
1413+
14101414
if osStack {
14111415
// Return from mstart and let the system thread
14121416
// library free the g0 stack and terminate the thread.

0 commit comments

Comments
 (0)