app crashes while deleting a wallet

[dreacot]

I encountered this error while deleting two wallets back to back.

I had 7 wallets, I then proceeded to delete one, which was successfully deleted, i then immediately proceeded to delete another one before the crash occurred

See log below:

2022-03-31 17:10:15.047 [INF] DLWL: Deleting Wallet
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x92f738]

goroutine 82 [running]:
[decred.org/dcrwallet/v2/wallet.(*Wallet).AccountBalance(0x0,](http://decred.org/dcrwallet/v2/wallet.(*Wallet).AccountBalance(0x0,) 0x1ed43d8, 0xc004d3c040, 0x200000000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/kennedy/Projects/pkg/mod/[decred.org/dcrwallet/[email protected]/wallet/wallet.go:1955](http://decred.org/dcrwallet/[email protected]/wallet/wallet.go:1955) +0xb8
[github.com/planetdecred/dcrlibwallet.(*Wallet).GetAccountBalance(0xc0001d8960,](http://github.com/planetdecred/dcrlibwallet.(*Wallet).GetAccountBalance(0xc0001d8960,) 0x0, 0x3, 0xc004bf4000, 0x0)
        /home/kennedy/Projects/pkg/mod/[github.com/planetdecred/[email protected]/accounts.go:108](http://github.com/planetdecred/[email protected]/accounts.go:108) +0x12f
[github.com/planetdecred/dcrlibwallet.(*Wallet).GetAccountsRaw(0xc0001d8960,](http://github.com/planetdecred/dcrlibwallet.(*Wallet).GetAccountsRaw(0xc0001d8960,) 0xc004d3c280, 0x0, 0x0)
        /home/kennedy/Projects/pkg/mod/[github.com/planetdecred/[email protected]/accounts.go:41](http://github.com/planetdecred/[email protected]/accounts.go:41) +0x13c
[github.com/planetdecred/godcr/ui/page.(*MainPage).CalculateTotalWalletsBalance(0xc001969800,](http://github.com/planetdecred/godcr/ui/page.(*MainPage).CalculateTotalWalletsBalance(0xc001969800,) 0xc00016c180, 0xc0004e1288, 0xc00195cdf0)
        /home/kennedy/Projects/src/[github.com/planetdecred/godcr/ui/page/main_page.go:309](http://github.com/planetdecred/godcr/ui/page/main_page.go:309) +0x9d
[github.com/planetdecred/godcr/ui/page.(*MainPage).updateBalance(0xc001969800)](http://github.com/planetdecred/godcr/ui/page.(*MainPage).updateBalance(0xc001969800))
        /home/kennedy/Projects/src/[github.com/planetdecred/godcr/ui/page/main_page.go:287](http://github.com/planetdecred/godcr/ui/page/main_page.go:287) +0x2f
[github.com/planetdecred/godcr/ui/page.(*MainPage).listenForNotifications.func1(0xc001969800)](http://github.com/planetdecred/godcr/ui/page.(*MainPage).listenForNotifications.func1(0xc001969800))
        /home/kennedy/Projects/src/[github.com/planetdecred/godcr/ui/page/main_page.go:880](http://github.com/planetdecred/godcr/ui/page/main_page.go:880) +0x1d7
created by [github.com/planetdecred/godcr/ui/page.(*MainPage).listenForNotifications](http://github.com/planetdecred/godcr/ui/page.(*MainPage).listenForNotifications)
        /home/kennedy/Projects/src/[github.com/planetdecred/godcr/ui/page/main_page.go:842](http://github.com/planetdecred/godcr/ui/page/main_page.go:842) +0x44f

[Sirmorrison]

I was unable to reproduce this issue while testing.

However, looking through the error, i see it happened here

Kindly confirm the circumstances around this process. Where you syncing the wallet before trying to delete it?

[dreacot]

my wallet was fully synced, but seeing as deleting a wallet disconnects from the network, and then connects back to the network after deleting, it's possible that the sync/re-connection process has started when i tried deleting the second wallet, as i did both of them immediately after the other

[LasTshaMAN]

Hello, I got interested and looked into it a bit, can suggest the following explanation for why this happens (would like your feedback on it, obviously :),

the issue is probably caused by accessing wallets map in un-sychronized way (without properly using mutexes/channels as is required by go memory model) from multiple different go-routines,

once user deletes his wallet in UI (as I understand the code, it happens here) from, lets call it, go-routine-1),

• this happens (lets mark it as **)
• and then wallets map gets updated: wallet item gets deleted

while from view-point of go-routine-2 (where executes the code, mentioned by @Sirmorrison above) wallet object is still present in wallets map and updateBalance gets called on it (while ** change has already been noticed by go-routine-2, but deletion from map hasn't been due to lack of synchorization between go-routines), and panics because of dereferencing w.db where w is nil here ...

The solution could be to synchronize access to the problematic map (and possibly badWallets map, because it likely suffers from the same issues) via a dedicated mutex, what do you think ?

At any rate, it seems to me MultiWallet is not safe (wallets part) for concurrent use, while it likely is meant to be.

[dreacot]

Hello, I got interested and looked into it a bit, can suggest the following explanation for why this happens (would like your feedback on it, obviously :),

the issue is probably caused by accessing wallets map in un-sychronized way (without properly using mutexes/channels as is required by go memory model) from multiple different go-routines,

once user deletes his wallet in UI (as I understand the code, it happens here) from, lets call it, go-routine-1),

• this happens (lets mark it as **)
• and then wallets map gets updated: wallet item gets deleted

while from view-point of go-routine-2 (where executes the code, mentioned by @Sirmorrison above) wallet object is still present in wallets map and updateBalance gets called on it (while ** change has already been noticed by go-routine-2, but deletion from map hasn't been due to lack of synchorization between go-routines), and panics because of dereferencing w.db where w is nil here ...

The solution could be to synchronize access to the problematic map (and possibly badWallets map, because it likely suffers from the same issues) via a dedicated mutex, what do you think ?

At any rate, it seems to me MultiWallet is not safe (wallets part) for concurrent use, while it likely is meant to be.

Thanks for sharing you idea, and also, tanks for painting a very vivid roadmap of your findings.

From your 4th paragraph, while from view-point of [go-routine-2](https://github.com/planetdecred/godcr/blob/b662b556246775ac103756b39ac7f593ae039c2f/ui/page/main_page.go#L821-L821)... it's true that the updateBalance method tries to reference a non existent map.

a solution would be worked on soon.

however, would you like to work on this?

[LasTshaMAN]

however, would you like to work on this?

From what I figured out, this should resolve it: planetdecred/dcrlibwallet#252

or do you have a different solution in mind ?

[github-actions]

No activity. Marking as stale