Skywiremob should work well if the socket is closed

[Senyoret1]

Describe the bug
For creating a mobile vpn client, Skywiremob is used to start a visor and then a socket is created in the mobile app to connect the mobile app with Skywiremob. In the case of the Android app, that is done creating a DatagramChannel in SkywireVPNConnection and then calling Skywiremob.SetMobileAppAddr(string). The problem is, if the socket is closed, Skywiremob panics and the app is force closed.

This should not happen, as there are multiple reasons that could cause the socket to be closed. One case is that in some circumstances the android app closes the socket and then tries to stop the visor, but as there are multiple treads the visor could try to use the socket just in the middle of the 2 operations and the panic makes the app to be force closed. Also, this problem may make it not possible to solve problems in which it is needed to recreate the socket.

Environment information:

• OS: Android 5 to 10 (x86 emulator).
• Platform: Android 5 to 10 (x86 emulator).

Steps to Reproduce
There is not easy way to make the problem happen. The code must be modified to close the socket while the visor continues running.

Actual behavior
The app is closed with an error like this:

E/GoLog: time="2020-09-04T19:13:41Z" level=error msg="Error resending traffic from VPN server to mobile app UDP conn" error="io: read/write on closed pipe"
E/Go: panic: send on closed channel
    goroutine 358 [running]:
E/Go: github.com/skycoin/skywire/pkg/transport.(*ManagedTransport).Serve.func3(0x94..)
    	C:/GoPath/src/github.com/skycoin/skywire/pkg/transport/managed_transport.go:166 +0x3d1
    created by github.com/skycoin/skywire/pkg/transport.(*ManagedTransport).Serve
    	C:/GoPath/src/github.com/skycoin/skywire/pkg/transport/managed_transport.go:146 +0x41f
A/libc: Fatal signal 6 (SIGABRT), code -6 (SI_TKILL) in tid 9058 (ire.skycoin.vpn), pid 9002 (ire.skycoin.vpn)
Disconnected from the target VM, address: 'localhost:8606', transport: 'socket'

Expected behavior
Skywiremob should not panic. Maybe it should continue working just like it does before the socket is created.

Additional context
No sure if totally related, but if the socket if closed, another one is created immediately after that and Skywiremob.SetMobileAppAddr(string) is called again, the following error appears:

panic: send on closed channel
    goroutine 18 [running, locked to thread]:
    github.com/skycoin/skywire/pkg/skywiremob.SetMobileAppAddr(0x81...)
    	C:/GoPath/src/github.com/skycoin/skywire/pkg/skywiremob/skywiremob.go:405 +0x175
    main.proxyskywiremob__SetMobileAppAddr(0xa3...)
    	C:/Users/User/AppData/Local/Temp/gomobile-work-669362907/src/gobind/go_skywiremobmain.go:94 +0x3e
    main._cgoexpwrap_b9c12c13c2ba_proxyskywiremob__SetMobileAppAddr(0xa3...)
    	_cgo_gotypes.go:303 +0x2a
A/libc: Fatal signal 6 (SIGABRT), code -6 in tid 9978 (RxNewThreadSche)

If it is not the same problem, it is also important to solve it, as the error prevents the ability to recover from some errors.

[Darkren]

hey there @Senyoret1 .

1. regarding what you've mentioned in additional context. I've added funcs to stop VPN client separately from the visor (StopVPNClient) and to close UDP socket (StopListeningUDP). so, in case socket fails, calling just SetMobileAppAddr won't result in panic, but it will hang. if socket fails, you need to:

• StopVPNClient
• StopListeningUDP
• PrepareVPNClient
• ShakeHands
• StartListeningUDP
• ServeVPN
• SetMobileAppAddr

we basically need to re-create the VPN client to get rid of spoiled socket. It should work, but please tell me if it doesn't.

haven't figured out the original issue yet, working on it

[Darkren]

@Senyoret1 could you please provide more info on how to reproduce the original issue? have been trying to close the tunnel in several different places, no panic so far. could you please check if panic exists with the current version of skywiremob and tell me how to reproduce it?

[Senyoret1]

Great, I made changes to the Android app and it is working well. About the first issue, I was unable to reproduce it again, so I’m not sure if it was solved with a change made to Skywiremob or the Android client, sorry about that.