mirror of
https://github.com/rivo/tview.git
synced 2026-04-26 13:25:51 +03:00
[GH-ISSUE #145] data races/synchronization #113
Labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/tview#113
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @3cb on GitHub (Jul 27, 2018).
Original GitHub issue: https://github.com/rivo/tview/issues/145
I'm having trouble synchronizing updates to Primitives and was hoping someone might be able to point me in the right direction.
My goal is to stream websocket data into multiple
Tableprimitives which are displayed one at a time. I'm using aListto switch between the Tables by adding and removing items from aFlex. Main.go gives a general idea of what I'm trying to do(https://github.com/3cb/cq/blob/master/main.go)I think the events triggered by the
Listprimitive are conflicting with the updates done in the goroutine's select block and don't know the simplest solution.@rivo commented on GitHub (Jul 28, 2018):
Without having run your code myself, I'm guessing that by pressing on a list item, you sometimes don't see the table change, correct? I believe this is because the activation of a list item causes
app.Draw()to be called immediately afterwards but your handling of that event happens in a separate goroutine. Most of the time, your goroutine will finish afterapp.Draw()has already been called. (Worst case, it messes up the execution ofapp.Draw()midway.)There are probably multiple ways to solve this. I would suggest that you make the
selectedcallback of theAddItem()function wait for the goroutine to finish processing the event. Something like this:Let me know if this helps.
@rivo commented on GitHub (Aug 4, 2018):
Will reopen if there is still an open issue here.
@3cb commented on GitHub (Aug 4, 2018):
Good catch. Incoming websocket data was triggering
app.Draw()so frequently that the problem was rarely noticeable. Unfortunately I'm still having a problem. Simply scrolling throughListitems without triggering theselectedcallback sets off the race detector: https://gist.github.com/3cb/89ecc2ba65a60280650611b41a537226Aside from adding mutexes to the Primitives, I'm thinking my only option would be to use
app.SetInputCapture()and instead of forwarding the key events I could handle them manually in the same goroutine as the other updates.@rivo commented on GitHub (Aug 7, 2018):
The call stack in your gist surprises me. The
app.Draw()function uses a mutex (see here) so it should never execute concurrently.I'm a bit puzzled as to how you're getting this race condition. Are you using the latest version of
tview?@3cb commented on GitHub (Aug 7, 2018):
I was a few commits behind. I think this commit eliminated part of the problem:
https://gist.github.com/3cb/9b847cebe16f363a232ce18b65b0c00d
The first four listed seem to be due to updating the
Tableprimitive directly in a separate goroutine beforeapp.Draw()is called:As I mentioned in the previous post the options I see are:
app.SetInputCapture()to intercept key events and handle updates toListprimitive in my own codeThe last race listed is interesting because
app.Run()seems to be conflicting withapp.Draw():The two options above would also solve this I think but I could be missing something easier. What are your thoughts?
@3cb commented on GitHub (Sep 18, 2018):
@rivo I just got back to this. I went ahead and added mutexes to the primitives I was using (
TableandList) as well asTableCellto correct the data races. Any interest in adding this to all primitives to make synchronization easier?@rivo commented on GitHub (Sep 25, 2018):
I started adding mutexes to these classes myself (I think I'd go for a slightly different solution than in your PR) but then stopped halfway. It adds a lot of boilerplate code to almost all functions throughout the package and I'm not sure it's worth it. In the applications I wrote myself using
tview, it was never needed.At the moment I'm considering putting a disclaimer into the package description itself that using the functions of the package in multiple goroutines will require outside synchronization.
So I'm a bit torn now. Which is to say that I haven't made up my mind yet.
What would it take for you to synchronize access to these objects in your application? Do you know any popular Go packages I could look at, that ensure thread-safety for all getters and setters (and other functions) in all objects?
@3cb commented on GitHub (Sep 25, 2018):
I actually haven't submitted a PR. I think you got mixed up with @joegrasse and his PR. Although we are both having problems syncing changes to
Tableit seems.For my application, all I did was add mutexes and then only updated the methods I use with lock/unlock; I haven't bothered with unused methods so I understand your hesitation to add all that code.
An alternative that I was thinking about is to add a channel to
Applicationtyped as anUpdaterinterface that would allow the user to inject updates to primitives into the event loop. What do you think about that?@rivo commented on GitHub (Sep 26, 2018):
Oops, right. I didn't realize that. I'll post this in the other thread, then.
I'm not so familiar with the
Updaterinterface. If you know a package that implements something like that, maybe you can point me to it?