use openvpn3 reconnect #590
Conversation
zolia
force-pushed
the
feature/MYST-580-use-openvpn3-reconnect
branch
from
284c789
to
4bc374b
Compare
zolia
force-pushed
the
feature/MYST-580-use-openvpn3-reconnect
branch
2 times, most recently
from
d45f50f
to
0d45af3
Compare
zolia
force-pushed
the
feature/MYST-580-use-openvpn3-reconnect
branch
from
0d45af3
to
c8b12a2
Compare
| } | ||
|
|
||
| type sessionTracker struct { | ||
| session *openvpn3.Session |
There was a problem hiding this comment.
This reference is being set/read from possibly different threads. Some kind of sync mechanism is needed, like Mutex
| func (st *sessionTracker) handleState(stateEvent connection.StateEvent) { | ||
| // On disconnected - remove session | ||
| if stateEvent.State == connection.Disconnecting { | ||
| st.session.Stop() |
There was a problem hiding this comment.
| st.session.Stop() | |
| st.session=nil |
| } | ||
|
|
||
| // Reconnect reconnects active session after given time | ||
| func (st *sessionTracker) Reconnect(afterSeconds int) error { |
There was a problem hiding this comment.
Maybe we can use here golang native time.Duration to avoid future conversion?
There was a problem hiding this comment.
C.reconnect accepts int, so no conversion is needed. From mobile frameworks, more straight type is int too.
|
|
||
| // Reconnect reconnects active session after given time | ||
| func (st *sessionTracker) Reconnect(afterSeconds int) error { | ||
| if st.session == nil { |
There was a problem hiding this comment.
Should we protect the reading of st.session if we are doing it in other places?
| return errors.New("session not created yet") | ||
| } | ||
|
|
||
| return st.session.Reconnect(afterSeconds) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Should we protect st.session if we are doing it in other places?
There was a problem hiding this comment.
wrapped st.session modification between mutex.
zolia
force-pushed
the
feature/MYST-580-use-openvpn3-reconnect
branch
from
70605c0
to
13ab5f6
Compare
| @@ -105,17 +105,21 @@ func (st *sessionTracker) sessionCreated(s *openvpn3.Session) { | |||
|
|
|||
| // Reconnect reconnects active session after given time | |||
| func (st *sessionTracker) Reconnect(afterSeconds int) error { | |||
| st.mux.Lock() | |||
| if st.session == nil { | |||
| return errors.New("session not created yet") | |||
There was a problem hiding this comment.
if we return here(when the st.session is null, for example), the st.mux will never unlock now.
zolia
force-pushed
the
feature/MYST-580-use-openvpn3-reconnect
branch
from
90f3341
to
4b2c9cf
Compare
No description provided.