openvpn_flutter icon indicating copy to clipboard operation
openvpn_flutter copied to clipboard

Multiple VPN Stage observers stacking on every connect() call

Open vedranivicatos opened this issue 2 years ago • 1 comments

When calling openVPN.connect() a new observer is created each time (ios/Classes/SwiftOpenVPNFlutterPlugin.swift :212):

NotificationCenter.default.addObserver(forName: NSNotification.Name.NEVPNStatusDidChange, object: nil , queue: nil) { notification in
                                    let nevpnconn = notification.object as! NEVPNConnection
                                    let status = nevpnconn.status
                                    self.onVpnStatusChanged(notification: status)
                                }

This object does not get disposed on disconnect() so on next call to connect() another observer will be created and so on. This leads to multiple events being sent to dart event listener instead of just a single one which causes unexpected and unwanted behaviour, memory leaks etc. There are multiple ways of fixing this:

  • explicitly removing the observer prior to adding a new one
  • explicitly removing the observer on disconnected event
  • modifying event stream for distinct events

I find the first approach is most likely the optimal one. I have made the changes locally and it works as it should.

                                if self.vpnStageObserver != nil {
                                    NotificationCenter.default.removeObserver(self.vpnStageObserver!, name: NSNotification.Name.NEVPNStatusDidChange, object: nil)
                                }
                                self.vpnStageObserver = NotificationCenter.default.addObserver(forName: NSNotification.Name.NEVPNStatusDidChange, object: nil , queue: nil) { [weak self] notification in
                                    let nevpnconn = notification.object as! NEVPNConnection
                                    let status = nevpnconn.status
                                    self?.onVpnStatusChanged(notification: status)
                                }

If I get the time I could fork the repo and make a PR.

vedranivicatos avatar Aug 29 '22 13:08 vedranivicatos

Can be closed as it's fixed in https://github.com/nizwar/openvpn_flutter/pull/45.

0ttik avatar Nov 23 '22 15:11 0ttik