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.

Sign up for GitHub

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

Jump to bottom

time: clean up duplicate definitions time.runtimeTimer and runtime.timer that need to be kept in sync #64549

Closed
j178 opened this issue Dec 5, 2023 · 5 comments
Closed

time: clean up duplicate definitions time.runtimeTimer and runtime.timer that need to be kept in sync #64549

j178 opened this issue Dec 5, 2023 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@j178
Copy link
Contributor

j178 commented Dec 5, 2023 •
edited

Go version

go version go1.21.4 darwin/amd64 and the tip

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

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GONOPROXY=''
GOOS='darwin'
GOPRIVATE=''
GOROOT='/usr/local/Cellar/go/1.21.4/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/Cellar/go/1.21.4/libexec/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.21.4'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
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/sw/0cn9x8611_xdn6c20pxd96m40000ks/T/go-build3138034287=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

In runtime/time.go:

go/src/runtime/time.go

Lines 16 to 40 in 098f059

// Package time knows the layout of this structure.
// If this struct changes, adjust ../time/sleep.go:/runtimeTimer.
type timer struct {
// If this timer is on a heap, which P's heap it is on.
// puintptr rather than *p to match uintptr in the versions
// of this struct defined in other packages.
pp puintptr
// Timer wakes up at when, and then at when+period, ... (period > 0 only)
// each time calling f(arg, now) in the timer goroutine, so f must be
// a well-behaved function and not block.
//
// when must be positive on an active timer.
when int64
period int64
f func(any, uintptr)
arg any
seq uintptr
// What to set the when field to in timerModifiedXX status.
nextwhen int64
// The status field holds one of the values below.
status atomic.Uint32
}

The status field was changed to atomic.Uint32, but not synced to time/sleep.go.

go/src/time/sleep.go

Lines 11 to 22 in 098f059

// Interface to timers implemented in package runtime.
// Must be in sync with ../runtime/time.go:/^type timer
type runtimeTimer struct {
pp uintptr
when int64
period int64
f func(any, uintptr) // NOTE: must not be closure
arg any
seq uintptr
nextwhen int64
status uint32
}

What did you expect to see?

Two structs are synced.

What did you see instead?

They are not.
@j178
Copy link
Contributor Author

j178 commented Dec 5, 2023

Related https://go-review.googlesource.com/c/go/+/428196 and #54865

@callthingsoff callthingsoff mentioned this issue Dec 5, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/547195 mentions this issue: time: make runtimeTimer.status atomic.Uint32 to sync with runtime.timer

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. labels Dec 5, 2023
@mknyszek mknyszek added this to the Backlog milestone Dec 6, 2023
@mknyszek mknyszek changed the title time: runtimeTimer is not synced with runtime.timer time: clean up duplicate definitions time.runtimeTimer and runtime.timer that need to be kept in sync Dec 6, 2023
qiulaidongfeng added a commit to qiulaidongfeng/go that referenced this issue Jan 29, 2024
@qiulaidongfeng
runtime,time: clean up duplicate definitions time.runtimeTimer and ru…
61e6fd1 
…ntime.timer

Fixex golang#64549

Change-Id: Ie6e46309c386e3186396115e20a0ee5b36891db0
@qiulaidongfeng qiulaidongfeng mentioned this issue Jan 29, 2024
Closed
@gopherbot
Copy link

Change https://go.dev/cl/559076 mentions this issue: runtime,time:: clean duplicate definitions time.runtimeTimer and runtime.timer

@qiulaidongfeng
Copy link
Contributor

qiulaidongfeng commented Jan 29, 2024 •
edited

See https://go.dev/cl/559076 , merge time.runtimeTimer and runtime.timer to abi.Timer , it's gonna make some weird race.
For example : https://storage.googleapis.com/go-build-log/2d213450/linux-amd64-newinliner_58fcafb2.log

@prattmic prattmic mentioned this issue Jan 29, 2024
Open
Jun10ng added a commit to Jun10ng/go that referenced this issue Feb 1, 2024
@Jun10ng
runtime: clean duplicate function
cb3351d 
Fixes golang#64549
For golang#65355
Jun10ng added a commit to Jun10ng/go that referenced this issue Feb 1, 2024
@Jun10ng
runtime: clean duplicate function
28c2ff5 
Fixes golang#64549
For golang#65355
qiulaidongfeng added a commit to qiulaidongfeng/go that referenced this issue Feb 22, 2024
@qiulaidongfeng
runtime,time: clean up duplicate definitions time.runtimeTimer and ru…
6ad3698 
…ntime.timer

Fixex golang#64549

Change-Id: Ie6e46309c386e3186396115e20a0ee5b36891db0
@qiulaidongfeng qiulaidongfeng mentioned this issue Mar 9, 2024
Closed
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 12, 2024
@dmitshur
Copy link
Contributor

This is fixed via CL 568339, which removed the duplication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Compiler / Runtime
Status: Done
Milestone
Backlog
6 participants
@mknyszek @dmitshur @bcmills @gopherbot @j178 @qiulaidongfeng