Godcr stuck in cancel sync #775
Conversation
| // Wallet notifications. Currently we are interested in reading from | ||
| // NotificationsUpdate channel, to prevent the channel from getting | ||
| // filled thereby blocking Wallet create/delete actions. |
There was a problem hiding this comment.
Seems to me that this is a bad design. Why do we have this channel in the first place and why do we need to drain it this way to prevent it from blocking?
There was a problem hiding this comment.
The channel provides sync status updates mainly used by the overview page. If it navigates away from the overview page the channel is no longer consumed, the channel gets filled up and block.
There was a problem hiding this comment.
I see use cases on the wallet page too. e.g Updating the status of operations on the UI, but that's beyond the scope of the current task.
There was a problem hiding this comment.
It'd be better for pages that require such updates to create the channel and close it when done with it. A universal channel that is not always used and could become blocked isn't ideal. I'll propose some changes in a moment.
There was a problem hiding this comment.
It'd be better for pages that require such updates to create the channel and close it when done with it. A universal channel that is not always used and could become blocked isn't ideal. I'll propose some changes in a moment.
I had thought about that, but a single universal channel is used to deliver tx/mixer/politeia/sync/rescan notifications. Changes will require breaking this up into units, so pages can create the units they need and close them on exit. This changes the scope of the task from fixing the bugs mentioned in the PR to refactoring the channels used to deliver notifications. Pointing that out before I proceed.
This PR close issue #728 and #765.