vincent/aptly
1
0
Fork 0
mirror of https://github.com/aptly-dev/aptly.git synced 2026-07-15 11:57:59 +00:00
Code Issues Projects Releases Wiki Activity

[adapted4debian] publish: fix race conditions

Browse Source 
* remove useless resource lock
  Resource locks need to be before the background task. creating same publish endpoint at the same time is unlikely...
* load data inside background tasks
  This fixes a flaw in async apis, which loaded the published repo from the DB and mutated it outside the task closure, before the task lock was acquired.
  Perform collection.LoadComplete inside maybeRunTaskInBackground and have tasks use a fresh copy of taskCollectionFactory, taskCollection
* lock source repos/snapshots for publish operations
  Concurrent tasks were not properly locking their resources, leading to inconsistent published indexes:
  SourceLocalRepo: iterate published.Sources (component -> source UUID), look up each local repo via localRepoCollection.ByUUID and append string(repo.Key()) to resources
  SourceSnapshot: iterate b.Snapshots,look up each snapshot via snapshotCollection.ByName and append string(snapshot.ResourceKey()) to resources.
* lock pool on non MultiDist publish
* revert mutex on LinkFromPool
* use uuids, since names can be renamed
* add test for MultiDist change
This commit is contained in:
André Roth
2026-05-20 13:40:37 +00:00
parent ebe3d55f9e
commit 8cba4eb06f
9 changed files with 1270 additions and 206 deletions
Download Patch File Download Diff File Expand all files Collapse all files
deb/publish.go
+11 -6
View File
@@ -604,6 +604,15 @@ func (p *PublishedRepo) Key() []byte {
return []byte("U" + p.StoragePrefix() + ">>" + p.Distribution)
}
// PrefixPoolLockKey returns the task-queue resource key that serialises all
// publish operations sharing the same pool directory under storagePrefix.
// It must be held whenever a non-MultiDist publish may read or clean the
// shared pool, to prevent concurrent cleanup runs from deleting each other's
// files. See docs/Resource-Locking.md for the full key-namespace table.
func PrefixPoolLockKey(storagePrefix string) string {
return "P" + storagePrefix
}
// RefKey is a unique id for package reference list
func (p *PublishedRepo) RefKey(component string) []byte {
return []byte("E" + p.UUID + component)
@@ -1127,7 +1136,7 @@ func (p *PublishedRepo) Publish(packagePool aptly.PackagePool, publishedStorageP
release["Label"] = p.GetLabel()
release["Suite"] = p.GetSuite()
release["Codename"] = p.GetCodename()
datetime_format := "Mon, 2 Jan 2006 15:04:05 MST"
datetimeFormat := "Mon, 2 Jan 2006 15:04:05 MST"
publishDate := time.Now().UTC()
if epoch := os.Getenv("SOURCE_DATE_EPOCH"); epoch != "" {
@@ -1135,15 +1144,11 @@ func (p *PublishedRepo) Publish(packagePool aptly.PackagePool, publishedStorageP
publishDate = time.Unix(sec, 0).UTC()
}
}
release["Date"] = publishDate.Format(datetime_format)
release["Date"] = publishDate.Format(datetimeFormat)
release["Architectures"] = strings.Join(utils.StrSlicesSubstract(p.Architectures, []string{ArchitectureSource}), " ")
if p.AcquireByHash {
release["Acquire-By-Hash"] = "yes"
}
if p.SignedBy != "" {
// Let's use a century as a "forever" value.
release["Valid-Until"] = publishDate.AddDate(100, 0, 0).Format(datetime_format)
}
release["Description"] = " Generated by aptly\n"
release["MD5Sum"] = ""
release["SHA1"] = ""
deb/publish_test.go
+11 -6
View File
@@ -436,14 +436,14 @@ func (s *PublishedRepoSuite) TestPublishNoSigner(c *C) {
func (s *PublishedRepoSuite) TestPublishSourceDateEpoch(c *C) {
// Test with SOURCE_DATE_EPOCH set
_ = os.Setenv("SOURCE_DATE_EPOCH", "1234567890")
defer os.Unsetenv("SOURCE_DATE_EPOCH")
defer func() { _ = os.Unsetenv("SOURCE_DATE_EPOCH") }()
err := s.repo.Publish(s.packagePool, s.provider, s.factory, &NullSigner{}, nil, false, "")
c.Assert(err, IsNil)
rf, err := os.Open(filepath.Join(s.publishedStorage.PublicPath(), "ppa/dists/squeeze/Release"))
c.Assert(err, IsNil)
defer rf.Close()
defer func() { _ = rf.Close() }()
cfr := NewControlFileReader(rf, true, false)
st, err := cfr.ReadStanza()
@@ -456,14 +456,14 @@ func (s *PublishedRepoSuite) TestPublishSourceDateEpoch(c *C) {
func (s *PublishedRepoSuite) TestPublishSourceDateEpochInvalid(c *C) {
// Test with invalid SOURCE_DATE_EPOCH (should fallback to current time)
_ = os.Setenv("SOURCE_DATE_EPOCH", "invalid")
defer os.Unsetenv("SOURCE_DATE_EPOCH")
defer func() { _ = os.Unsetenv("SOURCE_DATE_EPOCH") }()
err := s.repo2.Publish(s.packagePool, s.provider, s.factory, nil, nil, false, "")
c.Assert(err, IsNil)
rf, err := os.Open(filepath.Join(s.publishedStorage.PublicPath(), "ppa/dists/maverick/Release"))
c.Assert(err, IsNil)
defer rf.Close()
defer func() { _ = rf.Close() }()
cfr := NewControlFileReader(rf, true, false)
st, err := cfr.ReadStanza()
@@ -797,7 +797,10 @@ func (s *PublishedRepoCollectionSuite) TestListReferencedFiles(c *C) {
snap3 := NewSnapshotFromRefList("snap3", []*Snapshot{}, s.snap2.RefList(), "desc3")
_ = s.snapshotCollection.Add(snap3)
// Ensure that adding a second publish point with matching files doesn't give duplicate results.
// When a second publish point references the same package (snap3 is a clone of snap2,
// both containing p3/lonely-strangers), listReferencedFilesByComponent deduplicates by
// package ref so the file appears only once. StrSlicesSubstract handles a single entry
// correctly, so no duplicate is needed for cleanup safety.
repo3, err := NewPublishedRepo("", "", "anaconda-2", []string{}, []string{"main"}, []interface{}{snap3}, s.factory, false)
c.Check(err, IsNil)
c.Check(s.collection.Add(repo3), IsNil)
@@ -812,7 +815,9 @@ func (s *PublishedRepoCollectionSuite) TestListReferencedFiles(c *C) {
"a/alien-arena/alien-arena-common_7.40-2_i386.deb",
"a/alien-arena/mars-invaders_7.40-2_i386.deb",
},
"main": {"a/alien-arena/lonely-strangers_7.40-2_i386.deb"},
"main": {
"a/alien-arena/lonely-strangers_7.40-2_i386.deb",
},
})
}