1

I have this current function that originally was not context aware.

func (s *Service) ChunkUpload(r *multipart.Reader) error {
    chunk, err := s.parseChunk(r)
    if err != nil {
        return fmt.Errorf("failed parsing chunk %w", err)
    }

    if err := os.MkdirAll(chunk.UploadDir, 02750); err != nil {
        return err
    }

    if err := s.saveChunk(chunk); err != nil {
        return fmt.Errorf("failed saving chunk %w", err)
    }

    return nil
}

I've updated it's method call to now take a context.Context as its first argument. My main goal is to terminate and return the function as soon as the context is cancelled.

My initial implementation was this.

func (s *Service) ChunkUpload(ctx context.Context, r *multipart.Reader) error {
    errCh := make(chan error)

    go func() {
        chunk, err := s.parseChunk(r)
        if err != nil {
            errCh <- fmt.Errorf("failed parsing chunk %w", err)
            return
        }

        if err := os.MkdirAll(chunk.UploadDir, 02750); err != nil {
            errCh <- err
            return
        }

        if err := s.saveChunk(chunk); err != nil {
            errCh <- fmt.Errorf("failed saving chunk %w", err)
            return
        }
    }()

    select {
    case err := <-errCh:
        return err
    case <-ctx.Done():
        return ctx.Err()
    }
}

However, as I thought about the execution of the code I realized that this doesn't achieve my goal. Since all the function's logic is in a separate go routine even if context gets cancelled and I return ChunkUpload early the code within the go routine will continue to execute thus not really making a difference from the original code.

The next though was okay just pass a context to all inner functions like s.parseChunk and s.saveChunk but this option also doesn't seem right as I would need to implement cancellations in each function. What would be the proper way to refactor this original function to be context aware and terminate as soon as a context is cancelled?

icza
  • 389,944
  • 63
  • 907
  • 827
Rodrigo
  • 3,129
  • 3
  • 33
  • 61

2 Answers2

6

Function calls and goroutines cannot be terminated from the caller, the functions and goroutines have to support the cancellation, often via a context.Context value or a done channel.

In either case, the functions are responsible to check / monitor the context, and if cancel is requested (when the Context's done channel is closed), return early. There isn't an easier / automatic way.

If the task executes code in a loop, a convenient solution is to check the done channel in each iteration, and return if it's closed. If the task is one "monolith", the implementor is responsible to use / insert "checkpoints" at which the task can be reasonably aborted early if such cancellation is requested.

An easy way to check if the done channel is closed is to use a non-blocking select, such as:

select {
case <-ctx.Done():
    // Abort / return early
    return
default:
}

Care must be taken when the task uses other channel operations, as they may block in a nondeterministic way. Those selects should include the ctx.Done() channel too:

select {
case v := <- someChannel:
    // Do something with v
case <-ctx.Done():
    // Abort / return early
    return
}

Also be careful, because if the above receive from someChannel never blocks there is no guarantee a cancellation is properly handled, because if multiple communications can proceed in a select, one is chosen randomly (and there's no guarantee the <-ctx.Done() is ever chosen). In such case you may combine the above 2: first do a non-blocking check for cancellation, then use a select with your channel operations and the cancel monitoring.

icza
  • 389,944
  • 63
  • 907
  • 827
  • Thanks. I guess in my example I'm almost misusing context to try and terminate the function. By checkpoints do you mean doing something like this? https://gist.github.com/threeaccents/8690a5c0d2b900b73ca5a2180f578df4 – Rodrigo May 25 '20 at 13:13
  • @Rodrigo Yes, something like that. You should not "overdo" it, there's no reason to insert checks between operations that do not take time. Also, if there are many reasonable checkpoints, move that code into a closure to simplify code. – icza May 25 '20 at 13:16
  • Thanks the real answer to my question for this specific use case then is no need to use context since all operations are minimal. – Rodrigo May 25 '20 at 13:35
0

When we talked about canceling, we talked about a long run function or a block repeating multiple times, such as http.Serve()

As to your case, assume saveChunk will cost seconds to run, and you want to cancel when it's saving. So we can split the chunk into pieces, and save one by one, after each piece.

for i:=0;i<n;i++{
    select {
        case err := <- s.saveChunk(chunk[i]):
        {
             if err != nil {
                  fmt.Errorf("failed saving chunk %w", err)
                  return
              }
        }
        case <-ctx.Done():
              return
    }
}
Billy Yuan
  • 1,177
  • 7
  • 12