Skip to content

Commit bf4fd11

Browse files
ayushr2gvisor-bot
authored andcommitted
Make sandbox.Pid public and use it for Container.GoferPid.
The `pid` type in `sandbox.go` has been renamed to `Pid` and its methods have been exported. This `sandbox.Pid` type is now used for the `Container.GoferPid` field, ensurinr atomic access. All call sites accessing `GoferPid` have been updated to use atomics. Updates #12051 PiperOrigin-RevId: 803498368
1 parent 2f7f776 commit bf4fd11

File tree

6 files changed

+52
-48
lines changed

6 files changed

+52
-48
lines changed

runsc/cmd/capability_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func testCapabilities(t *testing.T, directfs bool) {
136136
if err := checkProcessCaps(c.Sandbox.Getpid(), wantSandboxCaps); err != nil {
137137
t.Error(err)
138138
}
139-
if err := checkProcessCaps(c.GoferPid, goferCaps); err != nil {
139+
if err := checkProcessCaps(c.GoferPid.Load(), goferCaps); err != nil {
140140
t.Error(err)
141141
}
142142
}

runsc/container/container.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ type Container struct {
118118

119119
// GoferPid is the PID of the gofer running along side the sandbox. May
120120
// be 0 if the gofer has been killed.
121-
GoferPid int `json:"goferPid"`
121+
GoferPid sandbox.Pid `json:"goferPid"`
122122

123123
// Sandbox is the sandbox this container is running in. It's set when the
124124
// container is created and reset when the sandbox is destroyed.
@@ -995,7 +995,7 @@ func (c *Container) createGoferFilestores(ovlConf config.Overlay2, mountHints *b
995995
// namespace, so that they don't prevent the host mount points from being
996996
// unmounted from the host's mount namespace. We will use /proc/pid/root
997997
// to access gofer's mount namespace. See proc_pid_root(5).
998-
goferRootfs := fmt.Sprintf("/proc/%d/root", c.GoferPid)
998+
goferRootfs := fmt.Sprintf("/proc/%d/root", c.GoferPid.Load())
999999

10001000
// Handle rootfs first.
10011001
rootfsConf := c.GoferMountConfs[0]
@@ -1129,11 +1129,11 @@ func (c *Container) stop() error {
11291129
}
11301130

11311131
// Try killing gofer if it does not exit with container.
1132-
if c.GoferPid != 0 {
1133-
log.Debugf("Killing gofer for container, cid: %s, PID: %d", c.ID, c.GoferPid)
1134-
if err := unix.Kill(c.GoferPid, unix.SIGKILL); err != nil {
1132+
if goferPid := c.GoferPid.Load(); goferPid != 0 {
1133+
log.Debugf("Killing gofer for container, cid: %s, PID: %d", c.ID, goferPid)
1134+
if err := unix.Kill(goferPid, unix.SIGKILL); err != nil {
11351135
// The gofer may already be stopped, log the error.
1136-
log.Warningf("Error sending signal %d to gofer %d: %v", unix.SIGKILL, c.GoferPid, err)
1136+
log.Warningf("Error sending signal %d to gofer %d: %v", unix.SIGKILL, goferPid, err)
11371137
}
11381138
}
11391139

@@ -1158,7 +1158,8 @@ func (c *Container) stop() error {
11581158
}
11591159

11601160
func (c *Container) waitForStopped() error {
1161-
if c.GoferPid == 0 {
1161+
goferPid := c.GoferPid.Load()
1162+
if goferPid == 0 {
11621163
return nil
11631164
}
11641165

@@ -1171,21 +1172,21 @@ func (c *Container) waitForStopped() error {
11711172
if c.goferIsChild {
11721173
// The gofer process is a child of the current process,
11731174
// so we can wait it and collect its zombie.
1174-
if _, err := unix.Wait4(int(c.GoferPid), nil, 0, nil); err != nil {
1175+
if _, err := unix.Wait4(int(goferPid), nil, 0, nil); err != nil {
11751176
return fmt.Errorf("error waiting the gofer process: %v", err)
11761177
}
1177-
c.GoferPid = 0
1178+
c.GoferPid.Store(0)
11781179
return nil
11791180
}
11801181

11811182
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
11821183
defer cancel()
11831184
b := backoff.WithContext(backoff.NewConstantBackOff(100*time.Millisecond), ctx)
11841185
op := func() error {
1185-
if err := unix.Kill(c.GoferPid, 0); err == nil {
1186+
if err := unix.Kill(goferPid, 0); err == nil {
11861187
return fmt.Errorf("gofer is still running")
11871188
}
1188-
c.GoferPid = 0
1189+
c.GoferPid.Store(0)
11891190
return nil
11901191
}
11911192
return backoff.Retry(op, b)
@@ -1435,7 +1436,7 @@ func (c *Container) createGoferProcess(conf *config.Config, mountHints *boot.Pod
14351436
return nil, nil, nil, nil, fmt.Errorf("gofer: %v", err)
14361437
}
14371438
log.Infof("Gofer started, PID: %d", cmd.Process.Pid)
1438-
c.GoferPid = cmd.Process.Pid
1439+
c.GoferPid.Store(cmd.Process.Pid)
14391440
c.goferIsChild = true
14401441
rpcPidCh <- cmd.Process.Pid
14411442

@@ -1448,7 +1449,7 @@ func (c *Container) createGoferProcess(conf *config.Config, mountHints *boot.Pod
14481449

14491450
// Set up nvproxy with the Gofer's mount namespaces while chrootSyncSandEnd is
14501451
// is still open.
1451-
if err := nvproxySetup(c.Spec, conf, c.GoferPid); err != nil {
1452+
if err := nvproxySetup(c.Spec, conf, c.GoferPid.Load()); err != nil {
14521453
return nil, nil, nil, nil, fmt.Errorf("setting up nvproxy for gofer: %w", err)
14531454
}
14541455

@@ -1567,10 +1568,11 @@ func runInCgroup(cg cgroup.Cgroup, fn func() error) error {
15671568

15681569
// adjustGoferOOMScoreAdj sets the oom_store_adj for the container's gofer.
15691570
func (c *Container) adjustGoferOOMScoreAdj() error {
1570-
if c.GoferPid == 0 || c.Spec.Process.OOMScoreAdj == nil {
1571+
goferPid := c.GoferPid.Load()
1572+
if goferPid == 0 || c.Spec.Process.OOMScoreAdj == nil {
15711573
return nil
15721574
}
1573-
return setOOMScoreAdj(c.GoferPid, *c.Spec.Process.OOMScoreAdj)
1575+
return setOOMScoreAdj(goferPid, *c.Spec.Process.OOMScoreAdj)
15741576
}
15751577

15761578
// adjustSandboxOOMScoreAdj sets the oom_score_adj for the sandbox.

runsc/container/container_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2084,7 +2084,7 @@ func TestGoferExits(t *testing.T) {
20842084
t.Fatalf("error killing sandbox process: %v", err)
20852085
}
20862086

2087-
err = blockUntilWaitable(c.GoferPid)
2087+
err = blockUntilWaitable(c.GoferPid.Load())
20882088
if err != nil && err != unix.ECHILD {
20892089
t.Errorf("error waiting for gofer to exit: %v", err)
20902090
}

runsc/container/multi_container_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,7 @@ func TestMultiContainerSignal(t *testing.T) {
837837
}
838838

839839
// goferPid is reset when container is destroyed.
840-
goferPid := containers[1].GoferPid
840+
goferPid := containers[1].GoferPid.Load()
841841

842842
// Destroy container and ensure container's gofer process has exited.
843843
if err := containers[1].Destroy(); err != nil {
@@ -867,7 +867,7 @@ func TestMultiContainerSignal(t *testing.T) {
867867
}
868868

869869
// Ensure that container's gofer and sandbox process are no more.
870-
err = blockUntilWaitable(containers[0].GoferPid)
870+
err = blockUntilWaitable(containers[0].GoferPid.Load())
871871
if err != nil && err != unix.ECHILD {
872872
t.Errorf("error waiting for gofer to exit: %v", err)
873873
}
@@ -1973,8 +1973,8 @@ func TestMultiContainerGoferKilled(t *testing.T) {
19731973
}
19741974

19751975
// Kill container's gofer.
1976-
if err := unix.Kill(c.GoferPid, unix.SIGKILL); err != nil {
1977-
t.Fatalf("unix.Kill(%d, SIGKILL)=%v", c.GoferPid, err)
1976+
if err := unix.Kill(c.GoferPid.Load(), unix.SIGKILL); err != nil {
1977+
t.Fatalf("unix.Kill(%d, SIGKILL)=%v", c.GoferPid.Load(), err)
19781978
}
19791979

19801980
// Wait until container stops.
@@ -2005,8 +2005,8 @@ func TestMultiContainerGoferKilled(t *testing.T) {
20052005

20062006
// Kill root container's gofer to bring entire sandbox down.
20072007
c = containers[0]
2008-
if err := unix.Kill(c.GoferPid, unix.SIGKILL); err != nil {
2009-
t.Fatalf("unix.Kill(%d, SIGKILL)=%v", c.GoferPid, err)
2008+
if err := unix.Kill(c.GoferPid.Load(), unix.SIGKILL); err != nil {
2009+
t.Fatalf("unix.Kill(%d, SIGKILL)=%v", c.GoferPid.Load(), err)
20102010
}
20112011

20122012
// Wait until sandbox stops. waitForProcessList will loop until sandbox exits

runsc/sandbox/sandbox.go

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -104,33 +104,35 @@ func createControlSocket(rootDir, id string) (string, int, error) {
104104
return "", -1, fmt.Errorf("unable to find location to write socket file")
105105
}
106106

107-
// pid is an atomic type that implements JSON marshal/unmarshal interfaces.
108-
type pid struct {
107+
// Pid is an atomic type that implements JSON marshal/unmarshal interfaces.
108+
type Pid struct {
109109
val atomicbitops.Int64
110110
}
111111

112-
func (p *pid) store(pid int) {
112+
// Store stores the PID in p.
113+
func (p *Pid) Store(pid int) {
113114
p.val.Store(int64(pid))
114115
}
115116

116-
func (p *pid) load() int {
117+
// Load loads the PID from p.
118+
func (p *Pid) Load() int {
117119
return int(p.val.Load())
118120
}
119121

120122
// UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON.
121-
func (p *pid) UnmarshalJSON(b []byte) error {
123+
func (p *Pid) UnmarshalJSON(b []byte) error {
122124
var pid int
123125

124126
if err := json.Unmarshal(b, &pid); err != nil {
125127
return err
126128
}
127-
p.store(pid)
129+
p.Store(pid)
128130
return nil
129131
}
130132

131133
// MarshalJSON implements json.Marshaler.MarshalJSON
132-
func (p *pid) MarshalJSON() ([]byte, error) {
133-
return json.Marshal(p.load())
134+
func (p *Pid) MarshalJSON() ([]byte, error) {
135+
return json.Marshal(p.Load())
134136
}
135137

136138
// Sandbox wraps a sandbox process.
@@ -156,7 +158,7 @@ type Sandbox struct {
156158

157159
// Pid is the pid of the running sandbox. May be 0 if the sandbox
158160
// is not running.
159-
Pid pid `json:"pid"`
161+
Pid Pid `json:"pid"`
160162

161163
// UID is the user ID in the parent namespace that the sandbox is running as.
162164
UID int `json:"uid"`
@@ -231,7 +233,7 @@ type Sandbox struct {
231233

232234
// Getpid returns the process ID of the sandbox process.
233235
func (s *Sandbox) Getpid() int {
234-
return s.Pid.load()
236+
return s.Pid.Load()
235237
}
236238

237239
// Args is used to configure a new sandbox.
@@ -388,7 +390,7 @@ func New(conf *config.Config, args *Args) (*Sandbox, error) {
388390

389391
// CreateSubcontainer creates a container inside the sandbox.
390392
func (s *Sandbox) CreateSubcontainer(conf *config.Config, cid string, tty *os.File) error {
391-
log.Debugf("Create sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.load())
393+
log.Debugf("Create sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.Load())
392394

393395
var files []*os.File
394396
if tty != nil {
@@ -428,7 +430,7 @@ func (s *Sandbox) StartRoot(conf *config.Config, spec *specs.Spec) error {
428430
if err := hostsettings.Handle(conf); err != nil {
429431
return fmt.Errorf("host settings: %w (use --host-settings=ignore to bypass)", err)
430432
}
431-
pid := s.Pid.load()
433+
pid := s.Pid.Load()
432434
log.Debugf("Start root sandbox %q, PID: %d", s.ID, pid)
433435
conn, err := s.sandboxConnect()
434436
if err != nil {
@@ -456,7 +458,7 @@ func (s *Sandbox) StartRoot(conf *config.Config, spec *specs.Spec) error {
456458

457459
// StartSubcontainer starts running a sub-container inside the sandbox.
458460
func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdios, goferFiles, goferFilestores []*os.File, devIOFile *os.File, goferConfs []boot.GoferMountConf) error {
459-
log.Debugf("Start sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.load())
461+
log.Debugf("Start sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.Load())
460462

461463
if err := s.configureStdios(conf, stdios); err != nil {
462464
return err
@@ -561,7 +563,7 @@ func (s *Sandbox) Restore(conf *config.Config, spec *specs.Spec, cid string, ima
561563
return err
562564
}
563565
// Configure the network.
564-
if err := setupNetwork(conn, s.Pid.load(), conf, disableIPv6); err != nil {
566+
if err := setupNetwork(conn, s.Pid.Load(), conf, disableIPv6); err != nil {
565567
return fmt.Errorf("setting up network: %v", err)
566568
}
567569

@@ -575,7 +577,7 @@ func (s *Sandbox) Restore(conf *config.Config, spec *specs.Spec, cid string, ima
575577

576578
// RestoreSubcontainer sends the restore call for a sub-container in the sandbox.
577579
func (s *Sandbox) RestoreSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdios, goferFiles, goferFilestoreFiles []*os.File, devIOFile *os.File, goferMountConf []boot.GoferMountConf) error {
578-
log.Debugf("Restore sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.load())
580+
log.Debugf("Restore sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.Load())
579581

580582
if err := s.configureStdios(conf, stdios); err != nil {
581583
return err
@@ -680,7 +682,7 @@ func (s *Sandbox) ProcfsDump() ([]procfs.ProcessProcfsDump, error) {
680682

681683
// NewCGroup returns the sandbox's Cgroup, or an error if it does not have one.
682684
func (s *Sandbox) NewCGroup() (cgroup.Cgroup, error) {
683-
return cgroup.NewFromPid(s.Pid.load(), false /* useSystemd */)
685+
return cgroup.NewFromPid(s.Pid.Load(), false /* useSystemd */)
684686
}
685687

686688
// Execute runs the specified command in the container. It returns the PID of
@@ -797,7 +799,7 @@ func (s *Sandbox) call(method string, arg, result any) error {
797799
}
798800

799801
func (s *Sandbox) connError(err error) error {
800-
return fmt.Errorf("connecting to control server at PID %d: %v", s.Pid.load(), err)
802+
return fmt.Errorf("connecting to control server at PID %d: %v", s.Pid.Load(), err)
801803
}
802804

803805
// createSandboxProcess starts the sandbox as a subprocess by running the "boot"
@@ -1287,7 +1289,7 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn
12871289
}
12881290

12891291
s.child = true
1290-
s.Pid.store(cmd.Process.Pid)
1292+
s.Pid.Store(cmd.Process.Pid)
12911293
log.Infof("Sandbox started, PID: %d", cmd.Process.Pid)
12921294

12931295
return nil
@@ -1390,7 +1392,7 @@ func (s *Sandbox) destroy() error {
13901392
log.Warningf("failed to delete control socket file %q: %v", controlSocketPath, err)
13911393
}
13921394
}
1393-
pid := s.Pid.load()
1395+
pid := s.Pid.Load()
13941396
if pid != 0 {
13951397
log.Debugf("Killing sandbox %q", s.ID)
13961398
if err := unix.Kill(pid, unix.SIGKILL); err != nil && err != unix.ESRCH {
@@ -1610,7 +1612,7 @@ func (s *Sandbox) ExportMetrics(opts control.MetricsExportOpts) (*prometheus.Sna
16101612

16111613
// IsRunning returns true if the sandbox or gofer process is running.
16121614
func (s *Sandbox) IsRunning() bool {
1613-
pid := s.Pid.load()
1615+
pid := s.Pid.Load()
16141616
if pid == 0 {
16151617
return false
16161618
}
@@ -1721,7 +1723,7 @@ func (s *Sandbox) waitForStopped() error {
17211723
if s.child {
17221724
s.statusMu.Lock()
17231725
defer s.statusMu.Unlock()
1724-
pid := s.Pid.load()
1726+
pid := s.Pid.Load()
17251727
if pid == 0 {
17261728
return nil
17271729
}
@@ -1730,7 +1732,7 @@ func (s *Sandbox) waitForStopped() error {
17301732
if _, err := unix.Wait4(int(pid), &s.status, 0, nil); err != nil {
17311733
return fmt.Errorf("error waiting the sandbox process: %v", err)
17321734
}
1733-
s.Pid.store(0)
1735+
s.Pid.Store(0)
17341736
return nil
17351737
}
17361738
ctx, cancel := context.WithTimeout(context.Background(), waitTimeout)
@@ -1878,7 +1880,7 @@ func (s *Sandbox) fixPidns(spec *specs.Spec) {
18781880
// pidns was not set, nothing to fix.
18791881
return
18801882
}
1881-
if pidns.Path != fmt.Sprintf("/proc/%d/ns/pid", s.Pid.load()) {
1883+
if pidns.Path != fmt.Sprintf("/proc/%d/ns/pid", s.Pid.Load()) {
18821884
// Fix only if the PID namespace corresponds to the sandbox's.
18831885
return
18841886
}

test/root/oom_score_adj_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func TestOOMScoreAdjSingle(t *testing.T) {
9090

9191
// Verify the gofer's oom_score_adj
9292
if testCase.OOMScoreAdj != nil {
93-
goferScore, err := specutils.GetOOMScoreAdj(c.GoferPid)
93+
goferScore, err := specutils.GetOOMScoreAdj(c.GoferPid.Load())
9494
if err != nil {
9595
t.Fatalf("error reading gofer oom_score_adj: %v", err)
9696
}
@@ -240,7 +240,7 @@ func TestOOMScoreAdjMulti(t *testing.T) {
240240
for i, c := range containers {
241241
if oomScoreAdj[i] != nil {
242242
// Verify the gofer's oom_score_adj
243-
score, err := specutils.GetOOMScoreAdj(c.GoferPid)
243+
score, err := specutils.GetOOMScoreAdj(c.GoferPid.Load())
244244
if err != nil {
245245
t.Fatalf("error reading gofer oom_score_adj: %v", err)
246246
}

0 commit comments

Comments
 (0)