Skip to content

Commit

Permalink
Revert "Avoid allocation storing last active time"
Browse files Browse the repository at this point in the history
This reverts commit edb6929.

In that commit, active time was changed from time.Time to
Unix time in order to avoid allocations. Unfortunately, that
has the side effect of discarding the monotonic component of
time.Time, and therefore makes our code vulnerable to stepping
of the system clock.

Fixes #697
  • Loading branch information
jech authored and paulwe committed May 26, 2024
1 parent 899594c commit 7e44037
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
16 changes: 8 additions & 8 deletions candidate_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ type candidateBase struct {

resolvedAddr net.Addr

lastSent atomic.Int64
lastReceived atomic.Int64
lastSent atomic.Value
lastReceived atomic.Value
conn net.PacketConn

currAgent *Agent
Expand Down Expand Up @@ -409,27 +409,27 @@ func (c *candidateBase) String() string {
// LastReceived returns a time.Time indicating the last time
// this candidate was received
func (c *candidateBase) LastReceived() time.Time {
if lastReceived := c.lastReceived.Load(); lastReceived != 0 {
return time.Unix(0, lastReceived)
if lastReceived, ok := c.lastReceived.Load().(time.Time); ok {
return lastReceived
}
return time.Time{}
}

func (c *candidateBase) setLastReceived(t time.Time) {
c.lastReceived.Store(t.UnixNano())
c.lastReceived.Store(t)
}

// LastSent returns a time.Time indicating the last time
// this candidate was sent
func (c *candidateBase) LastSent() time.Time {
if lastSent := c.lastSent.Load(); lastSent != 0 {
return time.Unix(0, lastSent)
if lastSent, ok := c.lastSent.Load().(time.Time); ok {
return lastSent
}
return time.Time{}
}

func (c *candidateBase) setLastSent(t time.Time) {
c.lastSent.Store(t.UnixNano())
c.lastSent.Store(t)
}

func (c *candidateBase) seen(outbound bool) {
Expand Down
4 changes: 2 additions & 2 deletions candidate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,15 @@ func TestCandidateLastSent(t *testing.T) {
require.Equal(t, candidate.LastSent(), time.Time{})
now := time.Now()
candidate.setLastSent(now)
require.EqualValues(t, 0, now.Sub(candidate.LastSent()))
require.Equal(t, candidate.LastSent(), now)
}

func TestCandidateLastReceived(t *testing.T) {
candidate := candidateBase{}
require.Equal(t, candidate.LastReceived(), time.Time{})
now := time.Now()
candidate.setLastReceived(now)
require.EqualValues(t, 0, now.Sub(candidate.LastReceived()))
require.Equal(t, candidate.LastReceived(), now)
}

func TestCandidateFoundation(t *testing.T) {
Expand Down

0 comments on commit 7e44037

Please sign in to comment.