[GH-ISSUE #241] Pass context.Context to Controlller #131

New issue

Closed

opened 2026-03-03 16:22:39 +03:00 by kerem · 4 comments

kerem commented 2026-03-03 16:22:39 +03:00

Owner

Copy link

Originally created by @flaviostutz on GitHub (Jul 29, 2020).
Original GitHub issue: https://github.com/mum4k/termdash/issues/241

Originally assigned to: @mum4k on GitHub.

I have a case where I need to control the "redraw" timing more accuratelly, so I am using Controller interface and "redraw()", but when closing the program, I am having some trouble and I think it is due to double closing the terminal due to the lack of context control.

I think it would be great for us to pass the context.Context to the controller so that one can control better the execution flow.

Using this code:

	controller, err := termdash.NewController(t, rootc, termdash.KeyboardSubscriber(evtHandler))
	if err != nil {
		panic(err)
	}

	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	ticker := time.NewTicker(time.Duration(opt.freq) * time.Second).C
	for {
		select {
		case <-ctx.Done():
			controller.Close()
			t.Close()
		case <-ticker:
			if !paused {
				err := curScreen.update(opt, ps)
				if err != nil {
					return
				}
				err = controller.Redraw()
			}
		}
	}

I have this log when exiting the application:

I wish I could use

	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	controller, err := termdash.NewController(ctx, t, rootc, termdash.KeyboardSubscriber(evtHandler))
	if err != nil {
		panic(err)
	}

	ticker := time.NewTicker(time.Duration(opt.freq) * time.Second).C
	for {
		select {
		case <-ctx.Done():
			controller.Close()
			t.Close()
		case <-ticker:
			if !paused {
				err := curScreen.update(opt, ps)
				if err != nil {
					return
				}
				err = controller.Redraw()
			}
		}
	}

Originally created by @flaviostutz on GitHub (Jul 29, 2020). Original GitHub issue: https://github.com/mum4k/termdash/issues/241 Originally assigned to: @mum4k on GitHub. I have a case where I need to control the "redraw" timing more accuratelly, so I am using Controller interface and "redraw()", but when closing the program, I am having some trouble and I think it is due to double closing the terminal due to the lack of context control. I think it would be great for us to pass the context.Context to the controller so that one can control better the execution flow. Using this code: ```golang controller, err := termdash.NewController(t, rootc, termdash.KeyboardSubscriber(evtHandler)) if err != nil { panic(err) } ctx, cancel := context.WithCancel(context.Background()) defer cancel() ticker := time.NewTicker(time.Duration(opt.freq) * time.Second).C for { select { case <-ctx.Done(): controller.Close() t.Close() case <-ticker: if !paused { err := curScreen.update(opt, ps) if err != nil { return } err = controller.Redraw() } } } ``` I have this log when exiting the application:  I wish I could use ```golang ctx, cancel := context.WithCancel(context.Background()) defer cancel() controller, err := termdash.NewController(ctx, t, rootc, termdash.KeyboardSubscriber(evtHandler)) if err != nil { panic(err) } ticker := time.NewTicker(time.Duration(opt.freq) * time.Second).C for { select { case <-ctx.Done(): controller.Close() t.Close() case <-ticker: if !paused { err := curScreen.update(opt, ps) if err != nil { return } err = controller.Redraw() } } } ```

kerem 2026-03-03 16:22:39 +03:00

• closed this issue
• added the
  question
  label

kerem commented 2026-03-03 16:22:40 +03:00

Author

Owner

Copy link

@mum4k commented on GitHub (Jul 29, 2020):

Hi @flaviostutz, the code snippets that you filed with the bug are only partial, so I had to guess the rest. I think I managed to recreate the problem. There seem to be two issues here and unless I am mistaken, they are both in the code you pasted. I.e. I didn't find a bug in the termdash code, but please feel free to correct me if I missed anything. Here's what I found:

1. panic: close of closed channel

This seems to be the result of calling Close() on the terminal twice. I don't see how you initialise the terminal in your code, but I am going to guess that it is using this code:

    t, err := termbox.New()
    if err != nil {
        panic(err)
    }
    defer t.Close()

If yes, the call to t.Close() is deferred above, but there is also a second call to t.Close() in the code snippet you linked, just under the call to controller.Close(). If that is the case, please try removing one of the two.

2. race on termination

This is unrelated and an observation that you might find useful. The ticker for loop doesn't actually terminate when the context closes. It continues to run and it has a chance to call another controller.Redraw() after the context closed and the controller closed. I would suggest breaking from the loop, the relevant portion:

    ticker := time.NewTicker(time.Duration(500 * time.Millisecond)).C
loop:
    for {
        select {
        case <-ctx.Done():
            controller.Close()
            break loop  // Ensures the loop won't execute again after we close the controller.
        case <-ticker:
            if err := controller.Redraw(); err != nil {
                panic(err)
            }
        }
    }

Alternatively you can avoid the ugly break label by placing the entire loop into a separate function and just returning instead of breaking.

Please let me know if this helped. If this didn't address the issue, please attach the complete code that will help me to reproduce and troubleshoot.

<!-- gh-comment-id:665935655 --> @mum4k commented on GitHub (Jul 29, 2020): Hi @flaviostutz, the code snippets that you filed with the bug are only partial, so I had to guess the rest. I think I managed to recreate the problem. There seem to be two issues here and unless I am mistaken, they are both in the code you pasted. I.e. I didn't find a bug in the termdash code, but please feel free to correct me if I missed anything. Here's what I found: 1) panic: close of closed channel This seems to be the result of calling Close() on the terminal twice. I don't see how you initialise the terminal in your code, but I am going to guess that it is using this code: ```go t, err := termbox.New() if err != nil { panic(err) } defer t.Close() ``` If yes, the call to `t.Close()` is deferred above, but there is also a second call to `t.Close()` in the code snippet you linked, just under the call to `controller.Close()`. If that is the case, please try removing one of the two. 2) race on termination This is unrelated and an observation that you might find useful. The ticker for loop doesn't actually terminate when the context closes. It continues to run and it has a chance to call another `controller.Redraw()` after the context closed and the controller closed. I would suggest breaking from the loop, the relevant portion: ```go ticker := time.NewTicker(time.Duration(500 * time.Millisecond)).C loop: for { select { case <-ctx.Done(): controller.Close() break loop // Ensures the loop won't execute again after we close the controller. case <-ticker: if err := controller.Redraw(); err != nil { panic(err) } } } ``` Alternatively you can avoid the ugly break label by placing the entire loop into a separate function and just returning instead of breaking. Please let me know if this helped. If this didn't address the issue, please attach the complete code that will help me to reproduce and troubleshoot.

kerem commented 2026-03-03 16:22:40 +03:00

Author

Owner

Copy link

@flaviostutz commented on GitHub (Jul 30, 2020):

Thanks, @mum4k, and sorry for the incomplete code I put at first. You did an excellent job analyzing this!

Let me put the whole code here for a better analysis. I made it work by "resolving" the error during close, but I think there is something wrong or with my code or with termdash.

Here it is:

package main

import (
	"context"
	"flag"
	"fmt"
	"time"

	"github.com/flaviostutz/perfstat"
	"github.com/flaviostutz/perfstat/detectors"
	"github.com/mum4k/termdash"
	"github.com/mum4k/termdash/container"
	"github.com/mum4k/termdash/keyboard"
	"github.com/mum4k/termdash/terminal/termbox"
	"github.com/mum4k/termdash/terminal/terminalapi"
	"github.com/sirupsen/logrus"
)

type Option struct {
	freq        float64
	sensibility float64
}

type screen interface {
	build(opt Option, ps *perfstat.Perfstat) (container.Option, error)
	update(opt Option, ps *perfstat.Perfstat, paused bool) error
	onEvent(evt *terminalapi.Keyboard)
}

var (
	opt                Option
	rootc              *container.Container
	ps                 *perfstat.Perfstat
	curScreenCtxCancel context.CancelFunc
	curScreen          screen
	paused             bool
)

func main() {

	flag.Float64Var(&opt.freq, "freq", 1.0, "Analysis frequency. Changes data capture and display refresh frequency. Higher consumes more CPU. Defaults to 1 Hz")
	flag.Float64Var(&opt.sensibility, "sensibility", 6.0, "Lower values (ex.: 0.2) means larger timespan in analysis, leading to more accurate results but slower responses. Higher values (ex.: 5) means short time analysis but may lead to false positives. Defaults to 1.0 which means detecting a continuous 100% CPU in 30s")
	flag.Parse()

	if opt.freq > 20 || opt.freq < 0.05 {
		panic("--freq must be between 0.05 and 20")
	}

	if opt.sensibility > 30 || opt.sensibility < 0.01 {
		panic("--sensibility must be between 0.01 and 30")
	}

	logrus.Debugf("Initializing Perfstat engine")
	opt2 := detectors.NewOptions()
	dur := time.Duration(30/opt.sensibility) * time.Second
	opt2.CPULoadAvgDuration = dur
	opt2.IORateLoadDuration = dur
	opt2.IOLimitsSpan = dur
	opt2.MemAvgDuration = dur
	opt2.MemLeakDuration = (dur * 10)

	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	ps = perfstat.Start(ctx, opt2)
	ps.SetLogLevel(logrus.DebugLevel)
	// time.Sleep(6 * time.Second)

	logrus.Debugf("Initializing UI...")

	t, err := termbox.New(termbox.ColorMode(terminalapi.ColorMode256))
	if err != nil {
		panic(err)
	}

	rootc, err = container.New(t, container.ID("root"))
	if err != nil {
		panic(err)
	}

	evtHandler := func(k *terminalapi.Keyboard) {
		if curScreen != nil {
			curScreen.onEvent(k)
		}
		//exit
		if k.Key == keyboard.KeyEsc || k.Key == keyboard.KeyCtrlC || k.Key == 113 {
			cancel()
			//pause/unpause
		} else if k.Key == 80 || k.Key == 112 {
			paused = !paused
		}
	}

	controller, err := termdash.NewController(t, rootc, termdash.KeyboardSubscriber(evtHandler))
	if err != nil {
		panic(err)
	}

	showScreen(&home{})

	paused = false

	ticker := time.NewTicker(time.Duration(opt.freq) * time.Second).C
	for {
		select {

		case <-ctx.Done():
			//this is used because when using controller some "double closing" arises
			//should be removed after fixing termdash (https://github.com/mum4k/termdash/issues/241)
			defer func() {
				recover()
			}()
			t.Close()

		case <-ticker:
			err := curScreen.update(opt, ps, paused)
			if err != nil {
				return
			}
			err = controller.Redraw()
		}
	}
}

func showScreen(s screen) {
	r, err := s.build(opt, ps)
	if err != nil {
		panic(fmt.Sprintf("Error preparing screen %s. err=%s", s, err))
	}
	rootc.Update("root", r)

	curScreen = s
}

See the whole project at https://github.com/flaviostutz/perfstat/blob/master/cli/main.go and thanks for your great work here!

<!-- gh-comment-id:666014267 --> @flaviostutz commented on GitHub (Jul 30, 2020): Thanks, @mum4k, and sorry for the incomplete code I put at first. You did an excellent job analyzing this! Let me put the whole code here for a better analysis. I made it work by "resolving" the error during close, but I think there is something wrong or with my code or with termdash. Here it is: ```golang package main import ( "context" "flag" "fmt" "time" "github.com/flaviostutz/perfstat" "github.com/flaviostutz/perfstat/detectors" "github.com/mum4k/termdash" "github.com/mum4k/termdash/container" "github.com/mum4k/termdash/keyboard" "github.com/mum4k/termdash/terminal/termbox" "github.com/mum4k/termdash/terminal/terminalapi" "github.com/sirupsen/logrus" ) type Option struct { freq float64 sensibility float64 } type screen interface { build(opt Option, ps *perfstat.Perfstat) (container.Option, error) update(opt Option, ps *perfstat.Perfstat, paused bool) error onEvent(evt *terminalapi.Keyboard) } var ( opt Option rootc *container.Container ps *perfstat.Perfstat curScreenCtxCancel context.CancelFunc curScreen screen paused bool ) func main() { flag.Float64Var(&opt.freq, "freq", 1.0, "Analysis frequency. Changes data capture and display refresh frequency. Higher consumes more CPU. Defaults to 1 Hz") flag.Float64Var(&opt.sensibility, "sensibility", 6.0, "Lower values (ex.: 0.2) means larger timespan in analysis, leading to more accurate results but slower responses. Higher values (ex.: 5) means short time analysis but may lead to false positives. Defaults to 1.0 which means detecting a continuous 100% CPU in 30s") flag.Parse() if opt.freq > 20 || opt.freq < 0.05 { panic("--freq must be between 0.05 and 20") } if opt.sensibility > 30 || opt.sensibility < 0.01 { panic("--sensibility must be between 0.01 and 30") } logrus.Debugf("Initializing Perfstat engine") opt2 := detectors.NewOptions() dur := time.Duration(30/opt.sensibility) * time.Second opt2.CPULoadAvgDuration = dur opt2.IORateLoadDuration = dur opt2.IOLimitsSpan = dur opt2.MemAvgDuration = dur opt2.MemLeakDuration = (dur * 10) ctx, cancel := context.WithCancel(context.Background()) defer cancel() ps = perfstat.Start(ctx, opt2) ps.SetLogLevel(logrus.DebugLevel) // time.Sleep(6 * time.Second) logrus.Debugf("Initializing UI...") t, err := termbox.New(termbox.ColorMode(terminalapi.ColorMode256)) if err != nil { panic(err) } rootc, err = container.New(t, container.ID("root")) if err != nil { panic(err) } evtHandler := func(k *terminalapi.Keyboard) { if curScreen != nil { curScreen.onEvent(k) } //exit if k.Key == keyboard.KeyEsc || k.Key == keyboard.KeyCtrlC || k.Key == 113 { cancel() //pause/unpause } else if k.Key == 80 || k.Key == 112 { paused = !paused } } controller, err := termdash.NewController(t, rootc, termdash.KeyboardSubscriber(evtHandler)) if err != nil { panic(err) } showScreen(&home{}) paused = false ticker := time.NewTicker(time.Duration(opt.freq) * time.Second).C for { select { case <-ctx.Done(): //this is used because when using controller some "double closing" arises //should be removed after fixing termdash (https://github.com/mum4k/termdash/issues/241) defer func() { recover() }() t.Close() case <-ticker: err := curScreen.update(opt, ps, paused) if err != nil { return } err = controller.Redraw() } } } func showScreen(s screen) { r, err := s.build(opt, ps) if err != nil { panic(fmt.Sprintf("Error preparing screen %s. err=%s", s, err)) } rootc.Update("root", r) curScreen = s } ``` See the whole project at https://github.com/flaviostutz/perfstat/blob/master/cli/main.go and thanks for your great work here!

kerem commented 2026-03-03 16:22:40 +03:00

Author

Owner

Copy link

@mum4k commented on GitHub (Jul 30, 2020):

Thank you for pasting the full code. Can you help me understand what you mean by " there is something wrong or with my code or with termdash"?

Is there an error message you can share or an error description?

<!-- gh-comment-id:666080480 --> @mum4k commented on GitHub (Jul 30, 2020): Thank you for pasting the full code. Can you help me understand what you mean by " there is something wrong or with my code or with termdash"? Is there an error message you can share or an error description?

kerem commented 2026-03-03 16:22:40 +03:00

Author

Owner

Copy link

@mum4k commented on GitHub (Nov 14, 2020):

Going to close this for inactivity, but please feel free to reopen if you would like to continue the discussion.

<!-- gh-comment-id:727162372 --> @mum4k commented on GitHub (Nov 14, 2020): Going to close this for inactivity, but please feel free to reopen if you would like to continue the discussion.

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

master

devel

lc_reset_example

dependabot/go_modules/devel/github.com/mattn/go-runewidth-0.0.16

release-0-20-0

grid_example

table

v0.20.0

v0.19.0

v0.18.0

v0.17.0

v0.16.1

v0.16.0

v0.15.0

v0.14.0

v0.13.0

v0.12.2

v0.12.1

v0.12.0

v0.11.0

v0.10.0

v0.9.1

v0.9.0

v0.8.0

v0.7.2

v0.7.1

v0.7.0

v0.6.1

v0.6.0

v0.5.0

v0.4.0

v0.3.0

v0.2.0

v0.1.0

Labels

Clear labels   

bug

  

cleanup

  

enhancement

  

enhancement

  

enhancement

  

good first issue

  

help wanted

  

help wanted

  

pull-request

Mirrored from GitHub Pull Request

  

question

  

wontfix

No labels

bug

cleanup

enhancement

enhancement

enhancement

good first issue

help wanted

help wanted

pull-request

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/termdash#131

No description provided.