Skip to content

GO-7561 acl: only the first consensus event finishes loading an acl object - #805

Merged
requilence merged 1 commit into
mainfrom
go-7561-acl-ready-once
Oct 7, 2026
Merged

requilence merged 1 commit into
mainfrom
go-7561-acl-ready-once

Conversation

@requilence

@requilence requilence commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Problem

newAclObject waits for the first consensus event of its watch. Both AddConsensusError and AddConsensusRecords closed ready while the store was still empty, so a watch that delivered more than one event before the object was dropped closed it twice:

panic: close of closed channel

This can happen in any service that uses acl.New() (coordinator, filenode, filenode2):

  • the consensus client hands a whole batch of events to the watcher while holding its mutex, so newAclObject can't unwatch between them;
  • a watch that times out and is watched again can receive the stale error of the first watch;
  • after GO-7561 concurrent AddLog of the same log returns ErrLogExists instead of WriteConflict any-sync-consensusnode#115, an UnWatch + Watch of a missing log while the client restores its watches after a reconnect (the restore is sent outside the client mutex) can get two errors for the same watcher; before, the server skipped the second request.

Records that arrived after records which failed to build a list also reached a nil list.

Fix

The first event finishes the load, once (finishLoad). After a failed load, later events are dropped; after a successful one, records are added as before and an error is only logged, now with the space id.

Tests

TestAclObject_EventsAfterTheFirst: error then records, two errors, and broken records then records panic on main; an error after the records leaves the object loaded.

Part of GO-7561. Must be deployed to the coordinator and both filenodes before anyproto/any-sync-consensusnode#115.

Release: services are on any-sync v0.13.6, and main after it also has #802 (GO-7556, net/pool and net/peer changes). Either tag a v0.13.6-based patch with only this change, or ship #802 together with it.

…bject

newAclObject waits for the first event of its watch. AddConsensusError and
AddConsensusRecords both closed ready while the store was still empty, so an
error followed by more events for the same watch closed it twice and
panicked. The consensus client delivers a batch of events to the watcher at
once, and a watch restored after a reconnect can report the same missing log
again, so the coordinator could crash.

The first event now finishes the load once; later events after a failed load
are dropped, and records after a successful load are added as before.
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

New Coverage 63.2% of statements
Patch Coverage 91.7% of changed statements (11/12)

Coverage provided by https://github.com/seriousben/go-patch-cover-action

@requilence
requilence merged commit 5d71cca into main Oct 7, 2026
4 checks passed
@requilence
requilence deleted the go-7561-acl-ready-once branch October 7, 2026 13:05
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants