8000 Race Condition on lock.lookupTime · Issue #184 · cirello-io/dynamolock · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Race Condition on lock.lookupTime #184

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
Closed
Areson opened this issue Mar 23, 2023 · 3 comments
Closed

Race Condition on lock.lookupTime #184

Areson opened this issue Mar 23, 2023 · 3 comments
Assignees
@ucirello
Labels
bug Something isn't working

Comments

@Areson
Copy link
Areson commented Mar 23, 2023

When a lock has both a heartbeat and a session monitor the lookupTime variable can be both written and read by multiple threads, creating a race condition.
@Areson Areson mentioned this issue Mar 23, 2023
Closed
@ucirello
Copy link
Collaborator 
==================
WARNING: DATA RACE
Write at 0x00c00036a0e8 by goroutine 57:
  cirello.io/dynamolock/v2.(*Lock).updateRVN()
      /go/src/github.com/cirello-io/dynamolock/v2/lock.go:93 +0x1544
  cirello.io/dynamolock/v2.(*Client).sendHeartbeat()
      /go/src/github.com/cirello-io/dynamolock/v2/client_heartbeat.go:134 +0x14f8
  cirello.io/dynamolock/v2.(*Client).SendHeartbeatWithContext()
      /go/src/github.com/cirello-io/dynamolock/v2/client_heartbeat.go:79 +0x154
  cirello.io/dynamolock/v2.(*Client).SendHeartbeat()
      /go/src/github.com/cirello-io/dynamolock/v2/client_heartbeat.go:60 +0x8c
  cirello.io/dynamolock/v2.(*Client).heartbeat.func1()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:692 +0x90
  sync.(*Map).Range()
      /usr/local/go/src/sync/map.go:476 +0x198
  cirello.io/dynamolock/v2.(*Client).heartbeat()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:690 +0x194
  cirello.io/dynamolock/v2.New.func2()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:135 +0x4c

Previous read at 0x00c00036a0e8 by goroutine 79:
  cirello.io/dynamolock/v2.(*Lock).timeUntilDangerZoneEntered()
      /go/src/github.com/cirello-io/dynamolock/v2/lock.go:149 +0xac
  cirello.io/dynamolock/v2.(*Client).lockSessionMonitorChecker.func1()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:1048 +0xf8

Goroutine 57 (running) created at:
  cirello.io/dynamolock/v2.New()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:135 +0x4e8
  cirello.io/dynamolock/v2_test.TestSessionMonitorRaceCondition()
      /go/src/github.com/cirello-io/dynamolock/v2/client_session_monitor_test.go:196 +0x204
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x180
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1629 +0x40

Goroutine 79 (running) created at:
  cirello.io/dynamolock/v2.(*Client).lockSessionMonitorChecker()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:1041 +0x158
  cirello.io/dynamolock/v2.(*Client).tryAddSessionMonitor()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:1022 +0xc0
  cirello.io/dynamolock/v2.(*Client).putLockItemAndStartSessionMonitor()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:586 +0x510
  cirello.io/dynamolock/v2.(*Client).upsertAndMonitorNewOrReleasedLock()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:550 +0xc14
  cirello.io/dynamolock/v2.(*Client).storeLock()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:414 +0xdbc
  cirello.io/dynamolock/v2.(*Client).acquireLock()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:350 +0x4c4
  cirello.io/dynamolock/v2.(*Client).AcquireLockWithContext()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:299 +0x16c
  cirello.io/dynamolock/v2.(*Client).AcquireLock()
      /go/src/github.com/cirello-io/dynamolock/v2/client.go:1110 +0x5e4
  cirello.io/dynamolock/v2_test.TestSessionMonitorRaceCondition()
      /go/src/github.com/cirello-io/dynamolock/v2/client_session_monitor_test.go:221 +0x584
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x180
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1629 +0x40
==================

ucirello added a commit that referenced this issue Mar 23, 2023
@ucirello
dynamolock/v2: add test to validate the bug
d4a93eb 
Addresses #184
@ucirello ucirello self-assigned this Mar 23, 2023
@ucirello ucirello added the bug Something isn't working label Mar 23, 2023
@Areson
Copy link
Author
Areson commented Mar 23, 2023

This causes the code to deadlock on itself, as timeUntilDangerZoneEntered calls IsExpired, which tries to also lock the semaphore. I think changing this to isExpired will fix.

@ucirello
Copy link
Collaborator

You're right! On it!

ucirello added a commit that referenced this issue Mar 23, 2023
@ucirello
dynamolock/v2: fix lock race condition
c540f30 
Addresses #184
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ucirello ucirello

Labels
bug Something isn't working
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Thread safe access to lookupTime
2 participants
@ucirello @Areson
0