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.

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

Missing lock in DeviceStatistics ("fatal error: concurrent map read and map write") #9274

Closed
syncthing-sentry bot opened this issue Dec 11, 2023 · 2 comments
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement)
Milestone

Comments

@syncthing-sentry
Copy link

INFO: syncthing v1.27.1-rc.1 "Gold Grasshopper" (go1.21.5 linux-amd64) builder@github.syncthing.net 2023-12-08 06:13:09 UTC
Panic at 2023-12-11T07:54:04+01:00
fatal error: concurrent map read and map write

goroutine 39 [running]:
github.com/syncthing/syncthing/lib/model.(*model).DeviceStatistics(0xc000002000)
	github.com/syncthing/syncthing/lib/model/model.go:821 +0x1c5
github.com/syncthing/syncthing/lib/connections.(*service).dialDevices(_, {_, _}, {_, _, _}, {0x25, {0xc00056f900, 0x3, 0x3}, ...}, ...)
	github.com/syncthing/syncthing/lib/connections/service.go:561 +0x142
github.com/syncthing/syncthing/lib/connections.(*service).connect(0xc00027d2c0, {0x1327f00, 0xc000362140})
	github.com/syncthing/syncthing/lib/connections/service.go:489 +0x22e
github.com/syncthing/syncthing/lib/svcutil.(*service).Serve(0xc000034740, {0x1327f00, 0xc000362140})
	github.com/syncthing/syncthing/lib/svcutil/svcutil.go:130 +0x85
github.com/thejerf/suture/v4.(*Supervisor).runService.func2()
	github.com/thejerf/suture/v4@v4.0.2/supervisor.go:565 +0xe6
created by github.com/thejerf/suture/v4.(*Supervisor).runService in goroutine 93
	github.com/thejerf/suture/v4@v4.0.2/supervisor.go:539 +0x1ca

//@calmh

@calmh calmh closed this as completed in 30fe2cf Dec 11, 2023
@calmh calmh added this to the v1.27.1 milestone Dec 11, 2023
calmh added a commit that referenced this issue Dec 11, 2023
Looking at deviceConnIDs requires this. Added in #9256.
calmh added a commit that referenced this issue Dec 11, 2023
* release:
  lib/model: Add pmut locking for DeviceStatistics (fixes #9274)
  lib/model: Remove spurious "replacing service" failure event (ref #9271)
@calmh calmh added the bug A problem with current functionality, as opposed to missing functionality (enhancement) label Dec 11, 2023
@calmh calmh modified the milestones: v1.27.1, v1.27.2 Dec 11, 2023
calmh added a commit to cjc7373/syncthing that referenced this issue Dec 11, 2023
* main: (69 commits)
  lib/nat: Fix test build failure (ref syncthing#9010)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/nat, lib/upnp: IPv6 UPnP support (syncthing#9010)
  gui, man, authors: Update docs, translations, and contributors
  gui: Show folder/device status on small screens (syncthing#8643)
  lib/model: Remove runner during folder cleanup (fixes syncthing#9269) (syncthing#9271)
  build: Update dependencies (syncthing#9265)
  build: Revert specifics for Go 1.21.4, build using Go 1.21.5 (syncthing#9264)
  lib/fs: Reduce memory usage in xattrs handling (syncthing#9251)
  lib/model: Improve LastSeen handling (syncthing#9256)
  lib/scanner: Record inode change time for directories and symlinks (syncthing#9250)
  lib/api: Improve ignore loading error handling (fixes syncthing#9253) (syncthing#9254)
  gui, man, authors: Update docs, translations, and contributors
  lib/fs: Better equality comparison in mtimefs
  cmd/stcrashreceiver: Add metrics for diskstore inventory
  cmd/stcrashreceiver: Minor cleanup, stricter file permissions
  cmd/stcrashreceiver: Add metrics for incoming reports
  ...
calmh added a commit to calmh/syncthing that referenced this issue Dec 16, 2023
* main: (89 commits)
  build: Update quic-go (fixes syncthing#9287)
  lib/model: Only handle relevant folder summaries (kqueue) (fixes syncthing#9183) (syncthing#9288)
  lib/model: Use a single lock (phase two: cleanup) (syncthing#9276)
  build(deps): bump actions/setup-go from 4 to 5 (syncthing#9279)
  lib/model: Use a single lock (syncthing#9275)
  cmd/syncthing: Better cli stdin handling (ref syncthing#9166) (syncthing#9281)
  cmd/syncthing: Mostly replace urfave/cli command line parser with alecthomas/kong (syncthing#9166)
  lib/nat: Fix test build failure (ref syncthing#9010)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/nat, lib/upnp: IPv6 UPnP support (syncthing#9010)
  gui, man, authors: Update docs, translations, and contributors
  gui: Show folder/device status on small screens (syncthing#8643)
  lib/model: Remove runner during folder cleanup (fixes syncthing#9269) (syncthing#9271)
  build: Update dependencies (syncthing#9265)
  build: Revert specifics for Go 1.21.4, build using Go 1.21.5 (syncthing#9264)
  lib/fs: Reduce memory usage in xattrs handling (syncthing#9251)
  lib/model: Improve LastSeen handling (syncthing#9256)
  ...
@MaxXor
Copy link

MaxXor commented Dec 19, 2023

Can you please release the update? @calmh It's crashing every day for me... and I don't want to disable usage survey (if that's related to the DeviceStatistics??)

@calmh
Copy link
Member

calmh commented Dec 19, 2023

@MaxXor excellent opportunity to check that the RC resolves it! https://github.com/syncthing/syncthing/releases/tag/v1.27.2-rc.1

(it's not the usage survey, so disabling that wouldn't help unfortunately)

calmh added a commit to calmh/syncthing that referenced this issue Jan 14, 2024
* main:
  lib/model: Use a single lock (phase two: cleanup) (syncthing#9276)
  build(deps): bump actions/setup-go from 4 to 5 (syncthing#9279)
  lib/model: Use a single lock (syncthing#9275)
  cmd/syncthing: Better cli stdin handling (ref syncthing#9166) (syncthing#9281)
  cmd/syncthing: Mostly replace urfave/cli command line parser with alecthomas/kong (syncthing#9166)
  lib/nat: Fix test build failure (ref syncthing#9010)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/nat, lib/upnp: IPv6 UPnP support (syncthing#9010)
  gui, man, authors: Update docs, translations, and contributors
  gui: Show folder/device status on small screens (syncthing#8643)
  lib/model: Remove runner during folder cleanup (fixes syncthing#9269) (syncthing#9271)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement)
Projects
None yet
Development

No branches or pull requests

2 participants