ronantakizawa/codereview-bench huggingface dataset:
Repository: celery/celery
File: t/integration/test_mem_leak_in_exception_handling.py (Python)

Code before:
"""
Integration tests for memory leak issue #8882.

These tests reproduce memory leak scenarios that occur when Celery tasks
raise unhandled exceptions, causing ExceptionInfo objects to not be
properly garbage collected.
"""

import gc
import os
import tracemalloc

from celery import Celery


class MemoryLeakUnhandledExceptionsTest:
    """Test class for memory leak scenarios with unhandled exceptions."""

    def __init__(self):
        self.app = Celery('test_memory_leak')
        self.app.conf.update(
            broker_url='memory://',
            result_backend='cache+memory://',
            task_always_eager=True,
            task_eager_propagates=True,
            task_store_eager_result=True,
        )
        self.setup_tasks()

    def setup_tasks(self):
        """Setup test tasks."""

        @self.app.task
        def task_success():
            """Task that completes successfully - baseline for memory comparison."""
            return "success"

        @self.app.task
        def task_unhandled_exception():
            """Task that raises an unhandled RuntimeError exception."""

Review comment (suggestion):
```suggestion
logger = logging.getLogger(__name__)

```

Code after:
"""
Integration tests for memory leak issue #8882.

These tests reproduce memory leak scenarios that occur when Celery tasks
raise unhandled exceptions, causing ExceptionInfo objects to not be
properly garbage collected.
"""

import gc
import logging
import os
import tracemalloc

from celery import Celery

logger = logging.getLogger(__name__)


class MemoryLeakUnhandledExceptionsTest:
    """Test class for memory leak scenarios with unhandled exceptions."""

    def __init__(self):
        self.app = Celery('test_memory_leak')
        self.app.conf.update(
            broker_url='memory://',
            result_backend='cache+memory://',
            task_always_eager=True,
            task_eager_propagates=True,
            task_store_eager_result=True,
        )
        self.setup_tasks()

    def setup_tasks(self):
        """Setup test tasks."""

        @self.app.task
        def task_success():
            """Task that completes successfully - baseline for memory comparison."""
            return "success"
Repository: charmbracelet/glow
File: main.go (Go)

Code before:
					// abort filepath.Walk
					return errors.New("source found")
				}
			}
			return nil
		})

		if src != nil {
			return src, nil
		}

		return nil, errors.New("missing markdown source")
	}

	// a file:
	r, err := os.Open(arg)
	u, _ := filepath.Abs(arg)
	return &source{r, u}, err
}

func validateOptions(cmd *cobra.Command) error {
	// grab config values from Viper
	width = viper.GetUint("width")
	localOnly = viper.GetBool("local")
	mouse = viper.GetBool("mouse")

	// validate the glamour style
	style = viper.GetString("style")
	if style != "auto" && glamour.DefaultStyles[style] == nil {
		style = utils.ExpandPath(style)
		if _, err := os.Stat(style); os.IsNotExist(err) {
			return fmt.Errorf("Specified style does not exist: %s", style)
		} else if err != nil {
			return err
		}
	}

	isTerminal := terminal.IsTerminal(int(os.Stdout.Fd()))
	// We want to use a special no-TTY style, when stdout is not a terminal
	// and there was no specific style passed by arg
	if !isTerminal && !cmd.Flags().Changed("style") {
		style = "notty"
	}

	// Detect terminal width
	if isTerminal && width == 0 && !cmd.Flags().Changed("width") {
		w, _, err := terminal.GetSize(int(os.Stdout.Fd()))
		if err == nil {
			width = uint(w)
		}


Review comment (suggestion):
I wonder if we should make this a `bool` or let the user specify the pager command as a string here. As it's currently in-line with how the `--pager` flag works, I think this is already an improvement I'll merge, though. Open for discussion nonetheless.

Code after:
					// abort filepath.Walk
					return errors.New("source found")
				}
			}
			return nil
		})

		if src != nil {
			return src, nil
		}

		return nil, errors.New("missing markdown source")
	}

	// a file:
	r, err := os.Open(arg)
	u, _ := filepath.Abs(arg)
	return &source{r, u}, err
}

func validateOptions(cmd *cobra.Command) error {
	// grab config values from Viper
	width = viper.GetUint("width")
	localOnly = viper.GetBool("local")
	mouse = viper.GetBool("mouse")
	pager = viper.GetBool("pager")

	// validate the glamour style
	style = viper.GetString("style")
	if style != "auto" && glamour.DefaultStyles[style] == nil {
		style = utils.ExpandPath(style)
		if _, err := os.Stat(style); os.IsNotExist(err) {
			return fmt.Errorf("Specified style does not exist: %s", style)
		} else if err != nil {
			return err
		}
	}

	isTerminal := terminal.IsTerminal(int(os.Stdout.Fd()))
	// We want to use a special no-TTY style, when stdout is not a terminal
	// and there was no specific style passed by arg
	if !isTerminal && !cmd.Flags().Changed("style") {
		style = "notty"
	}

	// Detect terminal width
	if isTerminal && width == 0 && !cmd.Flags().Changed("width") {
		w, _, err := terminal.GetSize(int(os.Stdout.Fd()))
		if err == nil {
			width = uint(w)
		}
Repository: charmbracelet/glow
File: ui/stash.go (Go)

Code before:
)

// MSG

type deletedStashedItemMsg int
type filteredMarkdownMsg []*markdown
type fetchedMarkdownMsg *markdown

type markdownFetchFailedMsg struct {
	err  error
	id   int
	note string
}

// MODEL

// stashViewState is the high-level state of the file listing.
type stashViewState int

const (
	stashStateReady stashViewState = iota
	stashStateLoadingDocument
	stashStateShowingError
)

func (s stashViewState) String() string {
	return map[stashViewState]string{
		stashStateReady:           "ready",
		stashStateLoadingDocument: "loading document",
		stashStateShowingError:    "showing error",
	}[s]
}

// The types of documents we are currently showing to the user.
type sectionKey int

const (
	localSection = iota
	stashedSection
	newsSection
	filterSection
)

func (s sectionKey) String() string {
	return map[sectionKey]string{
		localSection:   "local",
		stashedSection: "stashed",
		newsSection:    "news",
		filterSection:  "filtered",
	}[s]
}

Review comment (refactor):
Are these `String` functions actually used somewhere yet? I think we may be able to use them to clean up the code a bit.

Code after:
)

// MSG

type deletedStashedItemMsg int
type filteredMarkdownMsg []*markdown
type fetchedMarkdownMsg *markdown

type markdownFetchFailedMsg struct {
	err  error
	id   int
	note string
}

// MODEL

// stashViewState is the high-level state of the file listing.
type stashViewState int

const (
	stashStateReady stashViewState = iota
	stashStateLoadingDocument
	stashStateShowingError
)

// The types of documents we are currently showing to the user.
type sectionKey int

const (
	localSection = iota
	stashedSection
	newsSection
	filterSection
)

// section contains definitions and state information for displaying a tab and
// its contents in the file listing view.
type section struct {
	key       sectionKey
	docTypes  DocTypeSet
	paginator paginator.Model
	cursor    int
}

// map sections to their associated types.
var sections = map[sectionKey]section{
	localSection: {
		key:       localSection,
		docTypes:  NewDocTypeSet(LocalDoc),
		paginator: newStashPaginator(),
	},
Repository: charmbracelet/glow
File: glow_test.go (Go)

Code before:
package main

import (
	"bytes"
	"strings"
	"testing"
)

func TestGlowSources(t *testing.T) {
	tt := []string{
		".",
		"README.md",
		"github.com/charmbracelet/glow",
		"github://charmbracelet/glow",
		"https://github.com/charmbracelet/glow",
	}

	for _, v := range tt {
		t.Run(v, func(t *testing.T) {
			// Start by checking for network issues.
			_, nerr := readmeURL(v)
			if nerr != nil && strings.Contains(nerr.Error(), "no such host") {
				t.Logf("Error during execution (args: %s):\n%v", v, nerr)
				t.Skip("Test uses network. Are you connected to the Internet?")
			}

			buf := &bytes.Buffer{}
			err := executeArg(rootCmd, v, buf)
			if err != nil {
				t.Errorf("Error during execution (args: %s): %v", v, err)
			}
			if buf.Len() == 0 {
				t.Errorf("Output buffer should not be empty (args: %s)", v)
			}
		})
	}
}

func TestGlowFlags(t *testing.T) {
	tt := []struct {
		args  []string
		check func() bool
	}{
		{
			args: []string{"-p"},
			check: func() bool {
				return pager
			},
		},
		{

Review comment (suggestion):
Please try this, that would be safer

```suggestion
			// Start by checking for network issues.
			_, err := readmeURL(v)
			var dnsError *net.DNSError
			if errors.As(err, &dnsError) {
				t.Logf("Error during execution (args: %s):\n%v", v, err)
				t.Skip("Test uses network. Are you connected to the Internet?")
			}
```

This would be a more idiomatic way.

Code after:
package main

import (
	"testing"
)

func TestGlowFlags(t *testing.T) {
	tt := []struct {
		args  []string
		check func() bool
	}{
		{
			args: []string{"-p"},
			check: func() bool {
				return pager
			},
		},
		{
			args: []string{"-s", "light"},
			check: func() bool {
				return style == "light"
			},
		},
		{
			args: []string{"-w", "40"},
			check: func() bool {
				return width == 40
			},
		},
	}

	for _, v := range tt {
		err := rootCmd.ParseFlags(v.args)
		if err != nil {
			t.Fatal(err)
		}
		if !v.check() {
			t.Errorf("Parsing flag failed: %s", v.args)
		}
	}
}
Repository: charmbracelet/glow
File: glow_test.go (Go)

Code before:
	"bytes"
	"errors"
	"net"
	"testing"
)

func TestGlowSources(t *testing.T) {
	tt := []string{
		".",
		"README.md",
		"github.com/charmbracelet/glow",
		"github://charmbracelet/glow",
		"https://github.com/charmbracelet/glow",
	}

	for _, v := range tt {
		t.Run(v, func(t *testing.T) {
			var urlErr *net.DNSError
			buf := &bytes.Buffer{}
			_, err := sourceFromArg(v)
			if err != nil {
				// Check for network issues.
				if errors.As(err, &urlErr) {
					t.Logf("Error during execution (args: %s):\n%v", v, err)
					t.Skip("Test uses network. Are you connected to the Internet?")
				} else {
					t.Errorf("Error during execution (args: %s): %T %v", v, err, err)
				}

				if buf.Len() == 0 {
					t.Errorf("Output buffer should not be empty (args: %s)", v)
				}
			}
		})
	}
}

func TestGlowFlags(t *testing.T) {
	tt := []struct {
		args  []string
		check func() bool
	}{
		{
			args: []string{"-p"},
			check: func() bool {
				return pager
			},
		},
		{
			args: []string{"-s", "light"},
			check: func() bool {

Review comment (suggestion):
This could be easier to read

```suggestion
				// Check for network issues.
				if errors.As(err, &urlErr) {
					t.Logf("Error during execution (args: %s):\n%v", v, err)
					t.Skip("Test uses network. Are you connected to the Internet?")
					return
				}
				t.Errorf("Error during execution (args: %s): %T %v", v, err, err)

				if buf.Len() == 0 {
					t.Errorf("Output buffer should not be empty (args: %s)", v)
				}
```

Code after:
	"testing"
)

func TestGlowFlags(t *testing.T) {
	tt := []struct {
		args  []string
		check func() bool
	}{
		{
			args: []string{"-p"},
			check: func() bool {
				return pager
			},
		},
		{
			args: []string{"-s", "light"},
			check: func() bool {
				return style == "light"
			},
		},
		{
			args: []string{"-w", "40"},
			check: func() bool {
				return width == 40
			},
		},
	}

	for _, v := range tt {
		err := rootCmd.ParseFlags(v.args)
		if err != nil {
			t.Fatal(err)
		}
		if !v.check() {
			t.Errorf("Parsing flag failed: %s", v.args)
		}
	}
}
Repository: charmbracelet/glow
File: ui/ui.go (Go)

Code before:
	return batch
}

func newModel(cfg Config) tea.Model {
	initSections()

	if cfg.GlamourStyle == styles.AutoStyle {
		if te.HasDarkBackground() {
			cfg.GlamourStyle = styles.DarkStyle
		} else {
			cfg.GlamourStyle = styles.LightStyle
		}
	}

	common := commonModel{
		cfg: cfg,
	}

	m := model{
		common: &common,
		state:  stateShowStash,
		pager:  newPagerModel(&common),
		stash:  newStashModel(&common),
	}

	info, _ := os.Stat(cfg.Path)
	if info.IsDir() {
		m.state = stateShowStash
	} else {
		m.state = stateShowDocument
		m.pager.currentDocument = markdown{
			localPath: cfg.Path,
			Note:      stripAbsolutePath(cfg.Path, ""),
			Modtime:   info.ModTime(),
		}
	}

	return m
}

func (m model) Init() tea.Cmd {
	cmds := []tea.Cmd{m.stash.spinner.Tick}

	switch m.state {
	case stateShowStash:
		cmds = append(cmds, findLocalFiles(*m.common))
	case stateShowDocument:
		content, _ := os.ReadFile(m.common.cfg.Path)
		body := string(utils.RemoveFrontmatter(content))
		cmds = append(cmds, renderWithGlamour(m.pager, body))
	}

Review comment (bug):
Let me know what I could do on an error here given this function does not accept returning `err`.

Code after:
	return batch
}

func newModel(cfg Config) tea.Model {
	initSections()

	if cfg.GlamourStyle == styles.AutoStyle {
		if te.HasDarkBackground() {
			cfg.GlamourStyle = styles.DarkStyle
		} else {
			cfg.GlamourStyle = styles.LightStyle
		}
	}

	common := commonModel{
		cfg: cfg,
	}

	m := model{
		common: &common,
		state:  stateShowStash,
		pager:  newPagerModel(&common),
		stash:  newStashModel(&common),
	}

	info, err := os.Stat(cfg.Path)
	if err != nil {
		log.Error("unable to stat file", "file", m.common.cfg.Path, "error", err)
		m.fatalErr = err
		return m
	}
	if info.IsDir() {
		m.state = stateShowStash
	} else {
		cwd, _ := os.Getwd()
		m.state = stateShowDocument
		m.pager.currentDocument = markdown{
			localPath: cfg.Path,
			Note:      stripAbsolutePath(cfg.Path, cwd),
			Modtime:   info.ModTime(),
		}
	}

	return m
}

func (m model) Init() tea.Cmd {
	cmds := []tea.Cmd{m.stash.spinner.Tick}

	switch m.state {
	case stateShowStash:
Repository: charmbracelet/glow
File: ui/ui.go (Go)

Code before:
	}

	m := model{
		common: &common,
		state:  stateShowStash,
		pager:  newPagerModel(&common),
		stash:  newStashModel(&common),
	}

	info, _ := os.Stat(cfg.Path)
	if info.IsDir() {
		m.state = stateShowStash
	} else {
		m.state = stateShowDocument
		m.pager.currentDocument = markdown{
			localPath: cfg.Path,
			Note:      stripAbsolutePath(cfg.Path, ""),
			Modtime:   info.ModTime(),
		}
	}

	return m
}

func (m model) Init() tea.Cmd {
	cmds := []tea.Cmd{m.stash.spinner.Tick}

	switch m.state {
	case stateShowStash:
		cmds = append(cmds, findLocalFiles(*m.common))
	case stateShowDocument:
		content, _ := os.ReadFile(m.common.cfg.Path)
		body := string(utils.RemoveFrontmatter(content))
		cmds = append(cmds, renderWithGlamour(m.pager, body))
	}

	return tea.Batch(cmds...)
}

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	// If there's been an error, any key exits
	if m.fatalErr != nil {
		if _, ok := msg.(tea.KeyMsg); ok {
			return m, tea.Quit
		}
	}

	var cmds []tea.Cmd

	switch msg := msg.(type) {
	case tea.KeyMsg:

Review comment (suggestion):
Here `info` can be nil on error.
```suggestion
	info, err := os.Stat(cfg.Path)
	if err == nil {
	      if info.IsDir() {
		      m.state = stateShowStash
	      } else {
		      m.state = stateShowDocument
		      m.pager.currentDocument = markdown{
			      localPath: cfg.Path,
			      Note:      stripAbsolutePath(cfg.Path, ""),
			      Modtime:   info.ModTime(),
		      }
	      }
	}
```

Code after:
	}

	m := model{
		common: &common,
		state:  stateShowStash,
		pager:  newPagerModel(&common),
		stash:  newStashModel(&common),
	}

	info, err := os.Stat(cfg.Path)
	if err != nil {
		log.Error("unable to stat file", "file", m.common.cfg.Path, "error", err)
		m.fatalErr = err
		return m
	}
	if info.IsDir() {
		m.state = stateShowStash
	} else {
		cwd, _ := os.Getwd()
		m.state = stateShowDocument
		m.pager.currentDocument = markdown{
			localPath: cfg.Path,
			Note:      stripAbsolutePath(cfg.Path, cwd),
			Modtime:   info.ModTime(),
		}
	}

	return m
}

func (m model) Init() tea.Cmd {
	cmds := []tea.Cmd{m.stash.spinner.Tick}

	switch m.state {
	case stateShowStash:
		cmds = append(cmds, findLocalFiles(*m.common))
	case stateShowDocument:
		content, err := os.ReadFile(m.common.cfg.Path)
		if err != nil {
			log.Error("unable to read file", "file", m.common.cfg.Path, "error", err)
			return func() tea.Msg { return errMsg{err} }
		}
		body := string(utils.RemoveFrontmatter(content))
		cmds = append(cmds, renderWithGlamour(m.pager, body))
	}

	return tea.Batch(cmds...)
}

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	// If there's been an error, any key exits
Repository: charmbracelet/glow
File: ui/pager.go (Go)

Code before:

	if err := m.watcher.Add(path); err != nil {
		log.Error("error adding file to fsnotify watcher", "error", err)
		return nil
	}

	log.Info("fsnotify watching file", "file", path)

	for {
		select {
		case event, ok := <-m.watcher.Events:
			if !ok || !event.Has(fsnotify.Write) {
				continue
			}
			log.Debug("fsnotify event", "file", path, "event", event.Op)
			return reloadKeyMsg
		case err, ok := <-m.watcher.Errors:
			if !ok {
				continue
			}
			log.Debug("fsnotify error", "file", path, "error", err)
		}
	}
}

func (m *pagerModel) unwatchFile() {
	path := m.relFilePath()

	err := m.watcher.Remove(path)
	if err == nil {
		log.Debug("fsnotify file unwatched", "file", path)
	} else {
		log.Error("fsnotify fail to unwatch file", "file", path, "error", err)
	}
}

func (m *pagerModel) relFilePath() string {
	wd, _ := os.Getwd()
	return stripAbsolutePath(m.currentDocument.localPath, wd)
}


Review comment (suggestion):
The usual way to watch for these channels would be inside a goroutine. It wasn't clear for me how to address that though, as I don't know if there is any way to send a message from a remote context (another goroutine), specially considering this channel could receive multiple messages.

Given Bubbletea _appears_ to require an inline `return` here to force an update, I had to make this loop inline inside a `cmd`. Let me know if we have a better pattern for this, or if you have a better idea on how to implement it.

Code after:

func (m *pagerModel) watchFile() tea.Msg {
	path := m.relFilePath()

	if err := m.watcher.Add(path); err != nil {
		log.Error("error adding file to fsnotify watcher", "error", err)
		return nil
	}

	log.Info("fsnotify watching file", "file", path)

	for {
		select {
		case event, ok := <-m.watcher.Events:
			if !ok || !event.Has(fsnotify.Write) {
				continue
			}
			log.Debug("fsnotify event", "file", path, "event", event.Op)
			return reloadMsg{}
		case err, ok := <-m.watcher.Errors:
			if !ok {
				continue
			}
			log.Debug("fsnotify error", "file", path, "error", err)
		}
	}
}

func (m *pagerModel) unwatchFile() {
	path := m.relFilePath()

	err := m.watcher.Remove(path)
	if err == nil {
		log.Debug("fsnotify file unwatched", "file", path)
	} else {
		log.Error("fsnotify fail to unwatch file", "file", path, "error", err)
	}
}

func (m *pagerModel) relFilePath() string {
	wd, _ := os.Getwd()
	return stripAbsolutePath(m.currentDocument.localPath, wd)
}
Repository: charmbracelet/glow
File: ui/pager.go (Go)

Code before:
func (m *pagerModel) initWatcher() {
	var err error
	m.watcher, err = fsnotify.NewWatcher()
	if err != nil {
		log.Error("error creating fsnotify watcher", "error", err)
	}
}

func (m *pagerModel) watchFile() tea.Msg {
	path := m.relFilePath()

	if err := m.watcher.Add(path); err != nil {
		log.Error("error adding file to fsnotify watcher", "error", err)
		return nil
	}

	log.Info("fsnotify watching file", "file", path)

	for {
		select {
		case event, ok := <-m.watcher.Events:
			if !ok || !event.Has(fsnotify.Write) {
				continue
			}
			log.Debug("fsnotify event", "file", path, "event", event.Op)
			return reloadKeyMsg
		case err, ok := <-m.watcher.Errors:
			if !ok {
				continue
			}
			log.Debug("fsnotify error", "file", path, "error", err)
		}
	}
}

func (m *pagerModel) unwatchFile() {
	path := m.relFilePath()

	err := m.watcher.Remove(path)
	if err == nil {
		log.Debug("fsnotify file unwatched", "file", path)
	} else {
		log.Error("fsnotify fail to unwatch file", "file", path, "error", err)
	}
}

func (m *pagerModel) relFilePath() string {
	wd, _ := os.Getwd()
	return stripAbsolutePath(m.currentDocument.localPath, wd)
}


Review comment (suggestion):
i would probably make a `type reloadMsg struct{}` instead of triggering a key press event 

Code after:
	return content.String(), nil
}

func (m *pagerModel) initWatcher() {
	var err error
	m.watcher, err = fsnotify.NewWatcher()
	if err != nil {
		log.Error("error creating fsnotify watcher", "error", err)
	}
}

func (m *pagerModel) watchFile() tea.Msg {
	path := m.relFilePath()

	if err := m.watcher.Add(path); err != nil {
		log.Error("error adding file to fsnotify watcher", "error", err)
		return nil
	}

	log.Info("fsnotify watching file", "file", path)

	for {
		select {
		case event, ok := <-m.watcher.Events:
			if !ok || !event.Has(fsnotify.Write) {
				continue
			}
			log.Debug("fsnotify event", "file", path, "event", event.Op)
			return reloadMsg{}
		case err, ok := <-m.watcher.Errors:
			if !ok {
				continue
			}
			log.Debug("fsnotify error", "file", path, "error", err)
		}
	}
}

func (m *pagerModel) unwatchFile() {
	path := m.relFilePath()

	err := m.watcher.Remove(path)
	if err == nil {
		log.Debug("fsnotify file unwatched", "file", path)
	} else {
		log.Error("fsnotify fail to unwatch file", "file", path, "error", err)
	}
}

func (m *pagerModel) relFilePath() string {
	wd, _ := os.Getwd()