Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: CL 460476 introduced a leak on macos #62661

Open
kortschak opened this issue Sep 15, 2023 · 2 comments
Open

runtime: CL 460476 introduced a leak on macos #62661

kortschak opened this issue Sep 15, 2023 · 2 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Milestone

Comments

@kortschak
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.21.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE='on'
GOARCH='amd64'
GOBIN='/Users/user/go/bin'
GOCACHE='/Users/user/Library/Caches/go-build'
GOENV='/Users/user/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/user/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/user/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/user/goroot'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/user/goroot/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/user/thinking/leaks/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/0q/4mwk17890p70_n0q91m__v6w0000gn/T/go-build2891739865=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Run the program

package main

import "time"

func main() {
	for {
		time.Sleep(time.Second)
	}
}

as leaker and then run

$ leaks $(pgrep leaker) 2>/dev/null

What did you expect to see?

A clean leaks report.

$ leaks $(pgrep leaker) 2>/dev/null
Process:         leaker [81106]
Path:            /Users/USER/*/leaker
Load Address:    0x1000000
Identifier:      leaker
Version:         ???
Code Type:       X86-64
Platform:        macOS
Parent Process:  zsh [78427]

Date/Time:       2023-09-15 17:20:59.991 +0930
Launch Time:     2023-09-15 17:20:51.859 +0930
OS Version:      macOS 13.5.2 (22G91)
Report Version:  7
Analysis Tool:   /usr/bin/leaks

Physical footprint:         1040K
Physical footprint (peak):  1040K
Idle exit:                  untracked
----

leaks Report Version: 4.0
Process 81106: 176 nodes malloced for 12 KB
Process 81106: 0 leaks for 0 total leaked bytes.

What did you see instead?

$ leaks $(pgrep leaker) 2>/dev/null
Process:         leaker [80573]
Path:            /Users/USER/*/leaker
Load Address:    0x1000000
Identifier:      leaker
Version:         ???
Code Type:       X86-64
Platform:        macOS
Parent Process:  zsh [78427]

Date/Time:       2023-09-15 17:19:07.220 +0930
Launch Time:     2023-09-15 17:18:30.910 +0930
OS Version:      macOS 13.5.2 (22G91)
Report Version:  7
Analysis Tool:   /usr/bin/leaks

Physical footprint:         1060K
Physical footprint (peak):  1060K
Idle exit:                  untracked
----

leaks Report Version: 4.0
Process 80573: 185 nodes malloced for 14 KB
Process 80573: 1 leak for 32 total leaked bytes.

    1 (32 bytes) ROOT LEAK: <OS_xpc_date 0x600000204000> [32]

Bisected

76d39ae3499238ac7efb731f4f4cd47b1b3288ab is the first bad commit
commit 76d39ae3499238ac7efb731f4f4cd47b1b3288ab
Author: Russ Cox <rsc@golang.org>
Date:   Wed Jan 4 09:21:14 2023 -0500

    cmd/link, runtime: Apple libc atfork workaround take 3

    CL 451735 worked around bugs in Apple's atfork handlers by calling
    notify_is_valid_token and xpc_atfork_child at startup, so that init
    code that wouldn't be safe in the child process would be warmed up in
    the parent process instead, but xpc_atfork_child broke use of the xpc
    library in Go programs, and xpc is internally used by various macOS
    frameworks (#57263).

    CL 459175 reverted that change, and then CL 459176 tried a new
    approach: use __fork, which doesn't call any of the atfork handlers at all.
    That worked, but an Apple engineer reviewing the change in private
    email suggests that since __fork is not public API, it should be avoided.
    The same engineer (with access to the source code for the xpc library)
    suggests that the breakage in #57263 is caused by xpc_atfork_child
    marking the library as unusable, expecting an imminent call to exec,
    and that calling xpc_date_create_from_current instead would do the
    necessary initialization without marking xpc as unusable.

    CL 460475 reverted that change, to prepare for this one.

    This CL goes back to the original “call functions to warm things up”
    approach, replacing xpc_atfork_child with xpc_date_create_from_current.

    The CL also updates cmd/link to use OS and SDK version 10.13.0 for
    x86 macOS binaries, up from 10.9.0, also suggested by the Apple engineer.
    Combined with the two warmup calls, this makes the fork hangs go away.
    The minimum macOS version has been 10.13 High Sierra since Go 1.17,
    so there should be no problem with writing that in the binaries too.

    Fixes #33565.
    Fixes #56784.
    Fixes #57263.
    Fixes #57577.

    Change-Id: I20769d9daa1fe9ea930f8009481335f8a14dc21b
    Reviewed-on: https://go-review.googlesource.com/c/go/+/460476
    Auto-Submit: Russ Cox <rsc@golang.org>
    Run-TryBot: Russ Cox <rsc@golang.org>
    TryBot-Result: Gopher Robot <gobot@golang.org>
    Reviewed-by: Bryan Mills <bcmills@google.com>
    Reviewed-by: Cherry Mui <cherryyz@google.com>

 src/cmd/link/internal/ld/macho.go |  7 +++++--
 src/runtime/os_darwin.go          |  2 ++
 src/runtime/sys_darwin.go         | 42 +++++++++++++++++++++++++++++++++++++++
 src/runtime/sys_darwin_amd64.s    |  9 +++++++++
 src/runtime/sys_darwin_arm64.s    |  6 ++++++
 5 files changed, 64 insertions(+), 2 deletions(-)
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 15, 2023
@bcmills
Copy link
Contributor

bcmills commented Sep 15, 2023

Heh. We specifically discussed this leak in the code review but didn't expect that a leak-detector would actually diagnose it. 😅

(http://go.dev/cl/c/go/+/460476/5#message-5555c436e1a1b6a499d7211aba5e680e0a5503d3)

@bcmills bcmills changed the title runtime: 76d39ae introduces a leak on macos runtime: CL 460476 introduced a leak on macos Sep 15, 2023
@heschi heschi added this to the Go1.22 milestone Sep 15, 2023
@heschi heschi added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 15, 2023
@kortschak
Copy link
Contributor Author

The link for non-Googlers.

@gopherbot gopherbot modified the milestones: Go1.22, Go1.23 Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Projects
Development

No branches or pull requests

6 participants
@kortschak @prattmic @bcmills @gopherbot @heschi and others