Skip to content

Make TextView.Clear safe#641

Closed
sachaos wants to merge 1 commit intorivo:masterfrom
sachaos:master
Closed

Make TextView.Clear safe#641
sachaos wants to merge 1 commit intorivo:masterfrom
sachaos:master

Conversation

@sachaos
Copy link
Copy Markdown
Contributor

@sachaos sachaos commented Sep 9, 2021

Fix #636

I think the problem is TextView.Clear is not locking.
When we clear the TextView, the buffer will be nil.
So call Clear while drawing buffer, it will panic with that stack trace.

github.com/rivo/tview.(*TextView).Draw(0xc0000bc100, {0xa43700, 0xc000568000})
        /home/sam/go/pkg/mod/github.com/rivo/tview@v0.0.0-20210624165335-29d673af0ce2/textview.go:1017 +0xdf4

@rgrannell1
Copy link
Copy Markdown

+1, I've encountered similar issues that were resolved by locking the TextView

ctx.tgt.Lock()
ctx.tgt.Clear()
ctx.tgt.Unlock()

@SamWhited
Copy link
Copy Markdown
Contributor

ooooh, the TextView itself is lockable, I didn't realze that's something I could do. That will almost certainly fix it, thanks.

@SamWhited
Copy link
Copy Markdown
Contributor

I wanted to quickly follow up and request a different strategy: instead of merging this, let's remove the locking from around writes and leave it to the user. Otherwise if I want to clear a buffer and then do a bunch of writes I incur lock overhead each time. Instead I would prefer to lock at the beginning of my function, clear, do writes in a tight loop, etc. then unlock.

@rivo rivo mentioned this pull request Sep 23, 2021
@rgrannell1
Copy link
Copy Markdown

rgrannell1 commented Sep 23, 2021

if you take that approach, please document it clearly in the concurrency section of the wiki! Also this will be extremely non-back-compatible

@rivo
Copy link
Copy Markdown
Owner

rivo commented Oct 29, 2021

This will not be needed anymore.

@rivo rivo closed this Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

panic in long textview

4 participants