Skip to content

Commit

Permalink
fix: fix goroutine leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
Ryooooooga committed Jan 6, 2023
1 parent 1bb138c commit 00b9226
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 17 deletions.
34 changes: 24 additions & 10 deletions pkg/gui/confirmation_panel.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gui

import (
"context"
"fmt"
"strings"

Expand All @@ -16,8 +17,10 @@ import (
// This file is for the rendering of confirmation panels along with setting and handling associated
// keybindings.

func (gui *Gui) wrappedConfirmationFunction(function func() error) func() error {
func (gui *Gui) wrappedConfirmationFunction(cancel context.CancelFunc, function func() error) func() error {
return func() error {
cancel()

if err := gui.c.PopContext(); err != nil {
return err
}
Expand All @@ -32,8 +35,10 @@ func (gui *Gui) wrappedConfirmationFunction(function func() error) func() error
}
}

func (gui *Gui) wrappedPromptConfirmationFunction(function func(string) error, getResponse func() string) func() error {
func (gui *Gui) wrappedPromptConfirmationFunction(cancel context.CancelFunc, function func(string) error, getResponse func() string) func() error {
return func() error {
cancel()

if err := gui.c.PopContext(); err != nil {
return err
}
Expand Down Expand Up @@ -111,11 +116,12 @@ func (gui *Gui) getConfirmationPanelWidth() int {
}

func (gui *Gui) prepareConfirmationPanel(
ctx context.Context,
opts types.ConfirmOpts,
) error {
gui.Views.Confirmation.HasLoader = opts.HasLoader
if opts.HasLoader {
gui.g.StartTicking()
gui.g.StartTicking(ctx)
}
gui.Views.Confirmation.Title = opts.Title
// for now we do not support wrapping in our editor
Expand Down Expand Up @@ -145,23 +151,27 @@ func runeForMask(mask bool) rune {
return 0
}

func (gui *Gui) createPopupPanel(opts types.CreatePopupPanelOpts) error {
func (gui *Gui) createPopupPanel(ctx context.Context, opts types.CreatePopupPanelOpts) error {
gui.Mutexes.PopupMutex.Lock()
defer gui.Mutexes.PopupMutex.Unlock()

ctx, cancel := context.WithCancel(ctx)

// we don't allow interruptions of non-loader popups in case we get stuck somehow
// e.g. a credentials popup never gets its required user input so a process hangs
// forever.
// The proper solution is to have a queue of popup options
if gui.State.CurrentPopupOpts != nil && !gui.State.CurrentPopupOpts.HasLoader {
gui.Log.Error("ignoring create popup panel because a popup panel is already open")
cancel()
return nil
}

// remove any previous keybindings
gui.clearConfirmationViewKeyBindings()

err := gui.prepareConfirmationPanel(
ctx,
types.ConfirmOpts{
Title: opts.Title,
Prompt: opts.Prompt,
Expand All @@ -171,6 +181,7 @@ func (gui *Gui) createPopupPanel(opts types.CreatePopupPanelOpts) error {
Mask: opts.Mask,
})
if err != nil {
cancel()
return err
}
confirmationView := gui.Views.Confirmation
Expand All @@ -185,11 +196,13 @@ func (gui *Gui) createPopupPanel(opts types.CreatePopupPanelOpts) error {
confirmationView.RenderTextArea()
} else {
if err := gui.renderString(confirmationView, style.AttrBold.Sprint(opts.Prompt)); err != nil {
cancel()
return err
}
}

if err := gui.setKeyBindings(opts); err != nil {
if err := gui.setKeyBindings(cancel, opts); err != nil {
cancel()
return err
}

Expand All @@ -198,7 +211,7 @@ func (gui *Gui) createPopupPanel(opts types.CreatePopupPanelOpts) error {
return gui.c.PushContext(gui.State.Contexts.Confirmation)
}

func (gui *Gui) setKeyBindings(opts types.CreatePopupPanelOpts) error {
func (gui *Gui) setKeyBindings(cancel context.CancelFunc, opts types.CreatePopupPanelOpts) error {
actions := utils.ResolvePlaceholderString(
gui.c.Tr.CloseConfirm,
map[string]string{
Expand All @@ -210,13 +223,14 @@ func (gui *Gui) setKeyBindings(opts types.CreatePopupPanelOpts) error {
_ = gui.renderString(gui.Views.Options, actions)
var onConfirm func() error
if opts.HandleConfirmPrompt != nil {
onConfirm = gui.wrappedPromptConfirmationFunction(opts.HandleConfirmPrompt, func() string { return gui.Views.Confirmation.TextArea.GetContent() })
onConfirm = gui.wrappedPromptConfirmationFunction(cancel, opts.HandleConfirmPrompt, func() string { return gui.Views.Confirmation.TextArea.GetContent() })
} else {
onConfirm = gui.wrappedConfirmationFunction(opts.HandleConfirm)
onConfirm = gui.wrappedConfirmationFunction(cancel, opts.HandleConfirm)
}

keybindingConfig := gui.c.UserConfig.Keybinding
onSuggestionConfirm := gui.wrappedPromptConfirmationFunction(
cancel,
opts.HandleConfirmPrompt,
gui.getSelectedSuggestionValue,
)
Expand All @@ -235,7 +249,7 @@ func (gui *Gui) setKeyBindings(opts types.CreatePopupPanelOpts) error {
{
ViewName: "confirmation",
Key: keybindings.GetKey(keybindingConfig.Universal.Return),
Handler: gui.wrappedConfirmationFunction(opts.HandleClose),
Handler: gui.wrappedConfirmationFunction(cancel, opts.HandleClose),
},
{
ViewName: "confirmation",
Expand All @@ -260,7 +274,7 @@ func (gui *Gui) setKeyBindings(opts types.CreatePopupPanelOpts) error {
{
ViewName: "suggestions",
Key: keybindings.GetKey(keybindingConfig.Universal.Return),
Handler: gui.wrappedConfirmationFunction(opts.HandleClose),
Handler: gui.wrappedConfirmationFunction(cancel, opts.HandleClose),
},
{
ViewName: "suggestions",
Expand Down
20 changes: 13 additions & 7 deletions pkg/gui/popup/popup_handler.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package popup

import (
"context"
"strings"

"github.com/jesseduffield/gocui"
"github.com/jesseduffield/lazygit/pkg/common"
"github.com/jesseduffield/lazygit/pkg/gui/context"
gctx "github.com/jesseduffield/lazygit/pkg/gui/context"
"github.com/jesseduffield/lazygit/pkg/gui/style"
"github.com/jesseduffield/lazygit/pkg/gui/types"
"github.com/jesseduffield/lazygit/pkg/utils"
Expand All @@ -16,7 +17,7 @@ type PopupHandler struct {
*common.Common
index int
deadlock.Mutex
createPopupPanelFn func(types.CreatePopupPanelOpts) error
createPopupPanelFn func(context.Context, types.CreatePopupPanelOpts) error
onErrorFn func() error
popContextFn func() error
currentContextFn func() types.Context
Expand All @@ -30,7 +31,7 @@ var _ types.IPopupHandler = &PopupHandler{}

func NewPopupHandler(
common *common.Common,
createPopupPanelFn func(types.CreatePopupPanelOpts) error,
createPopupPanelFn func(context.Context, types.CreatePopupPanelOpts) error,
onErrorFn func() error,
popContextFn func() error,
currentContextFn func() types.Context,
Expand Down Expand Up @@ -96,7 +97,7 @@ func (self *PopupHandler) Confirm(opts types.ConfirmOpts) error {
self.index++
self.Unlock()

return self.createPopupPanelFn(types.CreatePopupPanelOpts{
return self.createPopupPanelFn(context.Background(), types.CreatePopupPanelOpts{
Title: opts.Title,
Prompt: opts.Prompt,
HandleConfirm: opts.HandleConfirm,
Expand All @@ -109,7 +110,7 @@ func (self *PopupHandler) Prompt(opts types.PromptOpts) error {
self.index++
self.Unlock()

return self.createPopupPanelFn(types.CreatePopupPanelOpts{
return self.createPopupPanelFn(context.Background(), types.CreatePopupPanelOpts{
Title: opts.Title,
Prompt: opts.InitialContent,
Editable: true,
Expand All @@ -127,12 +128,15 @@ func (self *PopupHandler) WithLoaderPanel(message string, f func() error) error
index = self.index
self.Unlock()

err := self.createPopupPanelFn(types.CreatePopupPanelOpts{
ctx, cancel := context.WithCancel(context.Background())

err := self.createPopupPanelFn(ctx, types.CreatePopupPanelOpts{
Prompt: message,
HasLoader: true,
})
if err != nil {
self.Log.Error(err)
cancel()
return nil
}

Expand All @@ -141,8 +145,10 @@ func (self *PopupHandler) WithLoaderPanel(message string, f func() error) error
self.Log.Error(err)
}

cancel()

self.Lock()
if index == self.index && self.currentContextFn().GetKey() == context.CONFIRMATION_CONTEXT_KEY {
if index == self.index && self.currentContextFn().GetKey() == gctx.CONFIRMATION_CONTEXT_KEY {
_ = self.popContextFn()
}
self.Unlock()
Expand Down

0 comments on commit 00b9226

Please sign in to comment.