Skip to content

Commit 345837b

Browse files
Ace-TangRandom-Liu
Ace-Tang
authored and
Random-Liu
committed
optimize shim lock in runtime v1
apply lock only around process map of shim service, avoid lock affect other procs operations. Signed-off-by: Ace-Tang <[email protected]>
1 parent 68a2cbc commit 345837b

1 file changed

Lines changed: 71 additions & 66 deletions

File tree

‎linux/shim/service.go‎

Lines changed: 71 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,9 @@ func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (*sh
159159

160160
// Start a process
161161
func (s *Service) Start(ctx context.Context, r *shimapi.StartRequest) (*shimapi.StartResponse, error) {
162-
s.mu.Lock()
163-
defer s.mu.Unlock()
164-
p := s.processes[r.ID]
165-
if p == nil {
166-
return nil, errdefs.ToGRPCf(errdefs.ErrNotFound, "process %s", r.ID)
162+
p, err := s.getExecProcess(r.ID)
163+
if err != nil {
164+
return nil, err
167165
}
168166
if err := p.Start(ctx); err != nil {
169167
return nil, err
@@ -176,16 +174,16 @@ func (s *Service) Start(ctx context.Context, r *shimapi.StartRequest) (*shimapi.
176174

177175
// Delete the initial process and container
178176
func (s *Service) Delete(ctx context.Context, r *ptypes.Empty) (*shimapi.DeleteResponse, error) {
179-
s.mu.Lock()
180-
defer s.mu.Unlock()
181-
p := s.processes[s.id]
182-
if p == nil {
183-
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
177+
p, err := s.getInitProcess()
178+
if err != nil {
179+
return nil, err
184180
}
185181
if err := p.Delete(ctx); err != nil {
186182
return nil, err
187183
}
184+
s.mu.Lock()
188185
delete(s.processes, s.id)
186+
s.mu.Unlock()
189187
s.platform.Close()
190188
return &shimapi.DeleteResponse{
191189
ExitStatus: uint32(p.ExitStatus()),
@@ -199,11 +197,9 @@ func (s *Service) DeleteProcess(ctx context.Context, r *shimapi.DeleteProcessReq
199197
if r.ID == s.id {
200198
return nil, status.Errorf(codes.InvalidArgument, "cannot delete init process with DeleteProcess")
201199
}
202-
s.mu.Lock()
203-
p := s.processes[r.ID]
204-
s.mu.Unlock()
205-
if p == nil {
206-
return nil, errors.Wrapf(errdefs.ErrNotFound, "process %s", r.ID)
200+
p, err := s.getExecProcess(r.ID)
201+
if err != nil {
202+
return nil, err
207203
}
208204
if err := p.Delete(ctx); err != nil {
209205
return nil, err
@@ -221,13 +217,14 @@ func (s *Service) DeleteProcess(ctx context.Context, r *shimapi.DeleteProcessReq
221217
// Exec an additional process inside the container
222218
func (s *Service) Exec(ctx context.Context, r *shimapi.ExecProcessRequest) (*ptypes.Empty, error) {
223219
s.mu.Lock()
224-
defer s.mu.Unlock()
225220

226221
if p := s.processes[r.ID]; p != nil {
222+
s.mu.Unlock()
227223
return nil, errdefs.ToGRPCf(errdefs.ErrAlreadyExists, "id %s", r.ID)
228224
}
229225

230226
p := s.processes[s.id]
227+
s.mu.Unlock()
231228
if p == nil {
232229
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
233230
}
@@ -243,22 +240,24 @@ func (s *Service) Exec(ctx context.Context, r *shimapi.ExecProcessRequest) (*pty
243240
if err != nil {
244241
return nil, errdefs.ToGRPC(err)
245242
}
243+
s.mu.Lock()
246244
s.processes[r.ID] = process
245+
s.mu.Unlock()
247246
return empty, nil
248247
}
249248

250249
// ResizePty of a process
251250
func (s *Service) ResizePty(ctx context.Context, r *shimapi.ResizePtyRequest) (*ptypes.Empty, error) {
252-
s.mu.Lock()
253-
defer s.mu.Unlock()
254251
if r.ID == "" {
255252
return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, "id not provided")
256253
}
257254
ws := console.WinSize{
258255
Width: uint16(r.Width),
259256
Height: uint16(r.Height),
260257
}
258+
s.mu.Lock()
261259
p := s.processes[r.ID]
260+
s.mu.Unlock()
262261
if p == nil {
263262
return nil, errors.Errorf("process does not exist %s", r.ID)
264263
}
@@ -270,11 +269,9 @@ func (s *Service) ResizePty(ctx context.Context, r *shimapi.ResizePtyRequest) (*
270269

271270
// State returns runtime state information for a process
272271
func (s *Service) State(ctx context.Context, r *shimapi.StateRequest) (*shimapi.StateResponse, error) {
273-
s.mu.Lock()
274-
defer s.mu.Unlock()
275-
p := s.processes[r.ID]
276-
if p == nil {
277-
return nil, errdefs.ToGRPCf(errdefs.ErrNotFound, "process id %s", r.ID)
272+
p, err := s.getExecProcess(r.ID)
273+
if err != nil {
274+
return nil, err
278275
}
279276
st, err := p.Status(ctx)
280277
if err != nil {
@@ -310,11 +307,9 @@ func (s *Service) State(ctx context.Context, r *shimapi.StateRequest) (*shimapi.
310307

311308
// Pause the container
312309
func (s *Service) Pause(ctx context.Context, r *ptypes.Empty) (*ptypes.Empty, error) {
313-
s.mu.Lock()
314-
defer s.mu.Unlock()
315-
p := s.processes[s.id]
316-
if p == nil {
317-
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
310+
p, err := s.getInitProcess()
311+
if err != nil {
312+
return nil, err
318313
}
319314
if err := p.(*proc.Init).Pause(ctx); err != nil {
320315
return nil, err
@@ -324,11 +319,9 @@ func (s *Service) Pause(ctx context.Context, r *ptypes.Empty) (*ptypes.Empty, er
324319

325320
// Resume the container
326321
func (s *Service) Resume(ctx context.Context, r *ptypes.Empty) (*ptypes.Empty, error) {
327-
s.mu.Lock()
328-
defer s.mu.Unlock()
329-
p := s.processes[s.id]
330-
if p == nil {
331-
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
322+
p, err := s.getInitProcess()
323+
if err != nil {
324+
return nil, err
332325
}
333326
if err := p.(*proc.Init).Resume(ctx); err != nil {
334327
return nil, err
@@ -338,22 +331,20 @@ func (s *Service) Resume(ctx context.Context, r *ptypes.Empty) (*ptypes.Empty, e
338331

339332
// Kill a process with the provided signal
340333
func (s *Service) Kill(ctx context.Context, r *shimapi.KillRequest) (*ptypes.Empty, error) {
341-
s.mu.Lock()
342-
defer s.mu.Unlock()
343334
if r.ID == "" {
344-
p := s.processes[s.id]
345-
if p == nil {
346-
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
335+
p, err := s.getInitProcess()
336+
if err != nil {
337+
return nil, err
347338
}
348339
if err := p.Kill(ctx, r.Signal, r.All); err != nil {
349340
return nil, errdefs.ToGRPC(err)
350341
}
351342
return empty, nil
352343
}
353344

354-
p := s.processes[r.ID]
355-
if p == nil {
356-
return nil, errdefs.ToGRPCf(errdefs.ErrNotFound, "process id %s not found", r.ID)
345+
p, err := s.getExecProcess(r.ID)
346+
if err != nil {
347+
return nil, err
357348
}
358349
if err := p.Kill(ctx, r.Signal, r.All); err != nil {
359350
return nil, errdefs.ToGRPC(err)
@@ -394,11 +385,9 @@ func (s *Service) ListPids(ctx context.Context, r *shimapi.ListPidsRequest) (*sh
394385

395386
// CloseIO of a process
396387
func (s *Service) CloseIO(ctx context.Context, r *shimapi.CloseIORequest) (*ptypes.Empty, error) {
397-
s.mu.Lock()
398-
defer s.mu.Unlock()
399-
p := s.processes[r.ID]
400-
if p == nil {
401-
return nil, errdefs.ToGRPCf(errdefs.ErrNotFound, "process does not exist %s", r.ID)
388+
p, err := s.getExecProcess(r.ID)
389+
if err != nil {
390+
return nil, err
402391
}
403392
if stdin := p.Stdin(); stdin != nil {
404393
if err := stdin.Close(); err != nil {
@@ -410,11 +399,9 @@ func (s *Service) CloseIO(ctx context.Context, r *shimapi.CloseIORequest) (*ptyp
410399

411400
// Checkpoint the container
412401
func (s *Service) Checkpoint(ctx context.Context, r *shimapi.CheckpointTaskRequest) (*ptypes.Empty, error) {
413-
s.mu.Lock()
414-
defer s.mu.Unlock()
415-
p := s.processes[s.id]
416-
if p == nil {
417-
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
402+
p, err := s.getInitProcess()
403+
if err != nil {
404+
return nil, err
418405
}
419406
if err := p.(*proc.Init).Checkpoint(ctx, &proc.CheckpointConfig{
420407
Path: r.Path,
@@ -434,11 +421,9 @@ func (s *Service) ShimInfo(ctx context.Context, r *ptypes.Empty) (*shimapi.ShimI
434421

435422
// Update a running container
436423
func (s *Service) Update(ctx context.Context, r *shimapi.UpdateTaskRequest) (*ptypes.Empty, error) {
437-
s.mu.Lock()
438-
defer s.mu.Unlock()
439-
p := s.processes[s.id]
440-
if p == nil {
441-
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
424+
p, err := s.getInitProcess()
425+
if err != nil {
426+
return nil, err
442427
}
443428
if err := p.(*proc.Init).Update(ctx, r.Resources); err != nil {
444429
return nil, errdefs.ToGRPC(err)
@@ -448,11 +433,9 @@ func (s *Service) Update(ctx context.Context, r *shimapi.UpdateTaskRequest) (*pt
448433

449434
// Wait for a process to exit
450435
func (s *Service) Wait(ctx context.Context, r *shimapi.WaitRequest) (*shimapi.WaitResponse, error) {
451-
s.mu.Lock()
452-
p := s.processes[r.ID]
453-
s.mu.Unlock()
454-
if p == nil {
455-
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
436+
p, err := s.getExecProcess(r.ID)
437+
if err != nil {
438+
return nil, err
456439
}
457440
p.Wait()
458441

@@ -494,11 +477,9 @@ func (s *Service) checkProcesses(e runc.Exit) {
494477
}
495478

496479
func (s *Service) getContainerPids(ctx context.Context, id string) ([]uint32, error) {
497-
s.mu.Lock()
498-
defer s.mu.Unlock()
499-
p := s.processes[s.id]
500-
if p == nil {
501-
return nil, errors.Wrapf(errdefs.ErrFailedPrecondition, "container must be created")
480+
p, err := s.getInitProcess()
481+
if err != nil {
482+
return nil, err
502483
}
503484

504485
ps, err := p.(*proc.Init).Runtime().Ps(ctx, id)
@@ -520,6 +501,30 @@ func (s *Service) forward(publisher events.Publisher) {
520501
}
521502
}
522503

504+
// getInitProcess returns initial process
505+
func (s *Service) getInitProcess() (proc.Process, error) {
506+
s.mu.Lock()
507+
defer s.mu.Unlock()
508+
509+
p := s.processes[s.id]
510+
if p == nil {
511+
return nil, errdefs.ToGRPCf(errdefs.ErrFailedPrecondition, "container must be created")
512+
}
513+
return p, nil
514+
}
515+
516+
// getExecProcess returns exec process
517+
func (s *Service) getExecProcess(id string) (proc.Process, error) {
518+
s.mu.Lock()
519+
defer s.mu.Unlock()
520+
521+
p := s.processes[id]
522+
if p == nil {
523+
return nil, errdefs.ToGRPCf(errdefs.ErrNotFound, "process %s does not exist", id)
524+
}
525+
return p, nil
526+
}
527+
523528
func getTopic(ctx context.Context, e interface{}) string {
524529
switch e.(type) {
525530
case *eventstypes.TaskCreate:

0 commit comments

Comments
 (0)