Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use openvpn3 reconnect #590

Merged
merged 5 commits into from Dec 4, 2018
Merged

use openvpn3 reconnect #590

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c8b12a2
use openvpn3 reconnect
zolia Nov 30, 2018
e9948d7
session usage made thread safe; session invalidation fixed
zolia Nov 30, 2018
13ab5f6
separate call to openvpn.Bootstrap()
zolia Dec 3, 2018
4b2c9cf
locking prevention for session usage
zolia Dec 3, 2018
afb684c
Merge branch 'master' into feature/MYST-580-use-openvpn3-reconnect
zolia Dec 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 3 additions & 5 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Gopkg.toml
View file
Open in desktop
Expand Up @@ -63,7 +63,7 @@

[[constraint]]
name = "github.com/mysteriumnetwork/go-openvpn"
version = "0.0.9"
version = "0.0.10"

[prune]
go-tests = true
Expand Down
48 changes: 45 additions & 3 deletions mobile/mysterium/openvpn_connection_setup.go
View file
Open in desktop
Expand Up @@ -18,9 +18,11 @@
package mysterium

import (
"errors"
"sync"

"github.com/cihub/seelog"
"github.com/mysteriumnetwork/go-openvpn/openvpn3"

"github.com/mysteriumnetwork/node/consumer"
"github.com/mysteriumnetwork/node/core/connection"
"github.com/mysteriumnetwork/node/services/openvpn"
Expand Down Expand Up @@ -85,8 +87,46 @@ func (adapter channelToCallbacksAdapter) OnStats(openvpnStats openvpn3.Statistic
// Openvpn3TunnelSetup is alias for openvpn3 tunnel setup interface exposed to Android/iOS interop
type Openvpn3TunnelSetup openvpn3.TunnelSetup

// OverrideOpenvpnConnection replaces default openvpn connection factory with mobile related one
func (mobNode *MobileNode) OverrideOpenvpnConnection(tunnelSetup Openvpn3TunnelSetup) {
// ReconnectableSession interface exposing reconnect for an active session
type ReconnectableSession interface {
Reconnect(afterSeconds int) error
}

type sessionTracker struct {
session *openvpn3.Session
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reference is being set/read from possibly different threads. Some kind of sync mechanism is needed, like Mutex

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, added mutex

mux sync.Mutex
}

func (st *sessionTracker) sessionCreated(s *openvpn3.Session) {
st.mux.Lock()
st.session = s
st.mux.Unlock()
}

// Reconnect reconnects active session after given time
func (st *sessionTracker) Reconnect(afterSeconds int) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use here golang native time.Duration to avoid future conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C.reconnect accepts int, so no conversion is needed. From mobile frameworks, more straight type is int too.

if st.session == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we protect the reading of st.session if we are doing it in other places?

zolia marked this conversation as resolved.
Show resolved Hide resolved
return errors.New("session not created yet")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we return here(when the st.session is null, for example), the st.mux will never unlock now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. Fixed.

}

return st.session.Reconnect(afterSeconds)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this can be run in a multithread way, st.session can become nil after check above.
I'd protect the whole body of the function with a mutex.

}

func (st *sessionTracker) handleState(stateEvent connection.StateEvent) {
// On disconnected - remove session
if stateEvent.State == connection.Disconnecting {
st.session = nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we protect st.session if we are doing it in other places?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrapped st.session modification between mutex.

zolia marked this conversation as resolved.
Show resolved Hide resolved
}
}

// OverrideOpenvpnConnection replaces default openvpn connection factory with mobile related one returning session that can be reconnected
func (mobNode *MobileNode) OverrideOpenvpnConnection(tunnelSetup Openvpn3TunnelSetup) ReconnectableSession {
openvpn.Bootstrap()

zolia marked this conversation as resolved.
Show resolved Hide resolved
st := &sessionTracker{}

mobNode.di.EventBus.Subscribe(connection.StateEventTopic, st.handleState)

mobNode.di.ConnectionRegistry.Register("openvpn", func(options connection.ConnectOptions, stateChannel connection.StateChannel, statisticsChannel connection.StatisticsChannel) (connection.Connection, error) {

vpnClientConfig, err := openvpn.NewClientConfigFromSession(options.SessionConfig, "", "")
Expand Down Expand Up @@ -117,9 +157,11 @@ func (mobNode *MobileNode) OverrideOpenvpnConnection(tunnelSetup Openvpn3TunnelS
}

session := openvpn3.NewMobileSession(config, credentials, channelToCallbacks(stateChannel, statisticsChannel), tunnelSetup)
st.sessionCreated(session)

return &sessionWrapper{
session: session,
}, nil
})
return st
}
2 changes: 1 addition & 1 deletion services/openvpn/bootstrap.go
View file
Open in desktop
Expand Up @@ -27,7 +27,7 @@ import (
// ServiceType indicates "openvpn" service type
const ServiceType = "openvpn"

// Bootstrap is called on program initialization time and registers various deserializers related to opepnvpn service
// Bootstrap is called on program initialization time and registers various deserializers related to OpenVPN service
func Bootstrap() {
dto_discovery.RegisterServiceDefinitionUnserializer(
ServiceType,
Expand Down