How can I completely terminate the running go func() when ctx times out?

Getting Help Code Review
1 
package main

import (
	"context"
	"log"
	"time"
)
func longRunningCalculation(timeCost int)chan string{
	result:=make(chan string)
	go func (){
		time.Sleep(time.Second*(time.Duration(timeCost)))
		log.Println("Still doing other things...")//Even if it times out, this goroutine is still doing other tasks.
		result<-"Done"
		log.Println(timeCost)
	}()
	return result
}
func jobWithTimeout(){
	ctx,cancel := context.WithTimeout(context.Background(), 2*time.Second)
	defer cancel()
	select{
	case <-ctx.Done():
		log.Println(ctx.Err())
		return
	case result:=<-longRunningCalculation(3):
		log.Println(result)
	}
}

func main() {
	jobWithTimeout()
	time.Sleep(time.Second*5)
}

What did you expect to see?

2019/09/25 11:00:16 context deadline exceeded

What did you see instead?

2019/09/25 11:00:16 context deadline exceeded
2019/09/25 11:00:17 Still doing other things…

1 Like
2

Add a Kill Channel

You need to pass another signal channel (kill) from jobWithTimeout into longRunningCalcuation and longRunningCalculation into that goroutine. The style is the same as select in jobWithTimeout.

Then when the case <-ctx.Done(), send the kill signal. The goroutines will handle the ending respectively.

EDIT: don’t go for this. Too much manual works.

Another Spotted Problem: Managing the Output

Have a channel controls the log output (log.Println). There should only be 1 function dealing with the log.Println reading from that channel.

1 Like
3

You should pass the context to the longRunningCalculation, and then you should do check cancelation there. But in some cases, you can’t escape from deadline exceeded, for example, if you are calling a library function and it does heavy operations and you have to wait for it or you should deal with the context deadline exceeded error. If you are doing some recursive or loop job, every recursive call or loop iteration you can check the context to decide whether it needs to be canceled.

1 Like
4

Another Spotted Problem: Managing the Output

No, this is bad, standard log library already deals with concurrent calls, no need to add extra synchronization.

2 Likes
5

Standard library is safe from concurrent calls but that does not means it guarantees output consistency. The decision factors is upto OP to control his/her output.

1 Like
6

I refactored your codes as I cannot see a clear concurrency mapping (e.g. who waits who, etc.). There are a few changes:

  1. jobWithTimeout makes more sense by scheduling a given job (can be any) from main.
  2. longRunningCalculation is an independent function.
  3. jobWithTimeout actually waits for the job to end (<-ctx.Done()) so main does not need an independent timeout.
  4. added a scheduled time log.Println("DEBUG: job scheduled") to cross-check the timing. You may remove it.
  5. jobWithTimeout log is independent from logRunningCalculation. I altered logRunningCalculation to show the differences.
  6. Apparently there is no way to distinguish between “cancel” and “done” for context.Context so if your function is done ahead of timeout, the expected has a “context cancelled” instead of “context completed/done”. I added the error message checking to filter off “context canceled”.

As @kync pointed out, this way of implementation does not always guarantees the deadline. That you need a different approach. Also, scrap my previous feedback since they are manual works using channels.

Refactored Codes:

package main

import (
	"context"
	"fmt"
	"log"
	"time"
)

func jobWithTimeout(limit int, job func(done context.CancelFunc)) {
	ctx, cancel := context.WithTimeout(context.Background(),
		time.Duration(limit)*time.Second)
	defer cancel()
	log.Println("DEBUG: job scheduled")
	go job(cancel)
	<-ctx.Done()
	err := ctx.Err()
	if err == nil || err.Error() == "context canceled" {
		return
	}
	log.Println(ctx.Err())
}

func longRunningCalculation(timeCost int) {
	time.Sleep(time.Second * (time.Duration(timeCost)))
	fmt.Println("Still doing other things...")
	fmt.Printf("%d\n", timeCost)
	fmt.Println("Done")
}

func main() {
	timeout := 2
	cost := 3

	jobWithTimeout(timeout, func(done context.CancelFunc) {
		longRunningCalculation(cost)
		done()
	})
}

// Output:
// 2019/09/27 18:27:49 DEBUG: job scheduled
// 2019/09/27 18:27:51 context deadline exceeded
2 Likes
7

But if i put the number of goroutines in the beginning and the end of the program i see 3 orphan goroutines still, is it a waste of resources ?

func main() {
timeout := 2
cost := 3
fmt.Println(runtime.NumGoroutine())
jobWithTimeout(timeout, func(done context.CancelFunc) {
longRunningCalculation(cost)
done()
})
fmt.Println(runtime.NumGoroutine())
}