aports/testing/kopia/fix-end-to-end-test.patch
2026-01-12 21:23:16 +00:00

399 lines
14 KiB
Diff

diff --git a/internal/bigmap/bigmap_32bit.go b/internal/bigmap/bigmap_32bit.go
new file mode 100644
index 00000000..44b5e4a6
--- /dev/null
+++ b/internal/bigmap/bigmap_32bit.go
@@ -0,0 +1,12 @@
+//go:build 386 || arm || mips || mipsle
+
+package bigmap
+
+// On 32-bit architectures, use smaller default memory parameters to avoid
+// running out of address space during garbage collection and other operations.
+const (
+ defaultNumMemorySegments = 4 // reduced from 8
+ defaultMemorySegmentSize = int64(4000000) // 4MB (reduced from 18MB)
+ defaultFileSegmentSize = 256 << 20 // 256 MiB (reduced from 1 GiB)
+ defaultInitialSizeLogarithm = 18 // reduced from 20
+)
diff --git a/internal/bigmap/bigmap_32bit_test.go b/internal/bigmap/bigmap_32bit_test.go
new file mode 100644
index 00000000..df945cd0
--- /dev/null
+++ b/internal/bigmap/bigmap_32bit_test.go
@@ -0,0 +1,18 @@
+//go:build 386 || arm || mips || mipsle
+
+package bigmap
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestDefaultConstants32Bit(t *testing.T) {
+ // Verify that 32-bit architectures use reduced memory parameters
+ // to avoid running out of address space during large operations
+ require.Equal(t, 4, defaultNumMemorySegments, "32-bit should use 4 memory segments")
+ require.Equal(t, int64(4*1000000), defaultMemorySegmentSize, "32-bit should use 4MB segments")
+ require.Equal(t, 256<<20, defaultFileSegmentSize, "32-bit should use 256MB file segments")
+ require.Equal(t, 18, defaultInitialSizeLogarithm, "32-bit should use initial size 2^18")
+}
diff --git a/internal/bigmap/bigmap_64bit.go b/internal/bigmap/bigmap_64bit.go
new file mode 100644
index 00000000..bcc0f829
--- /dev/null
+++ b/internal/bigmap/bigmap_64bit.go
@@ -0,0 +1,12 @@
+//go:build !386 && !arm && !mips && !mipsle
+
+package bigmap
+
+// On 64-bit architectures, use default memory parameters optimized for
+// large address spaces.
+const (
+ defaultNumMemorySegments = 8 // number of segments to keep in RAM
+ defaultMemorySegmentSize = int64(18000000) // 18MB enough to store >1M 16-17-byte keys
+ defaultFileSegmentSize = 1024 << 20 // 1 GiB
+ defaultInitialSizeLogarithm = 20
+)
diff --git a/internal/bigmap/bigmap_64bit_test.go b/internal/bigmap/bigmap_64bit_test.go
new file mode 100644
index 00000000..1ba076f2
--- /dev/null
+++ b/internal/bigmap/bigmap_64bit_test.go
@@ -0,0 +1,17 @@
+//go:build !386 && !arm && !mips && !mipsle
+
+package bigmap
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestDefaultConstants64Bit(t *testing.T) {
+ // Verify that 64-bit architectures use standard memory parameters
+ require.Equal(t, 8, defaultNumMemorySegments, "64-bit should use 8 memory segments")
+ require.Equal(t, int64(18*1000000), defaultMemorySegmentSize, "64-bit should use 18MB segments")
+ require.Equal(t, 1024<<20, defaultFileSegmentSize, "64-bit should use 1GB file segments")
+ require.Equal(t, 20, defaultInitialSizeLogarithm, "64-bit should use initial size 2^20")
+}
diff --git a/internal/bigmap/bigmap_internal.go b/internal/bigmap/bigmap_internal.go
index c1ed80d4..38e50841 100644
--- a/internal/bigmap/bigmap_internal.go
+++ b/internal/bigmap/bigmap_internal.go
@@ -24,10 +24,9 @@
)
const (
- defaultNumMemorySegments = 8 // number of segments to keep in RAM
- defaultMemorySegmentSize = 18 * 1e6 // 18MB enough to store >1M 16-17-byte keys
- defaultFileSegmentSize = 1024 << 20 // 1 GiB
- defaultInitialSizeLogarithm = 20
+ // defaultNumMemorySegments, defaultMemorySegmentSize, defaultFileSegmentSize,
+ // and defaultInitialSizeLogarithm are defined in bigmap_32bit.go and bigmap_64bit.go
+ // to use appropriate values for different architectures.
// grow hash table above this percentage utilization, higher values (close to 100) will be very slow,
// smaller values will waste memory.
diff --git a/internal/crypto/pb_key_deriver_scrypt.go b/internal/crypto/pb_key_deriver_scrypt.go
index ed0e82bc..a48c1739 100644
--- a/internal/crypto/pb_key_deriver_scrypt.go
+++ b/internal/crypto/pb_key_deriver_scrypt.go
@@ -6,9 +6,21 @@
)
const (
+ // scryptCostParameterN, scryptCostParameterR, and scryptCostParameterP
+ // are defined in pb_key_deriver_scrypt_32bit.go and pb_key_deriver_scrypt_64bit.go
+ // with architecture-specific values to avoid OOM on 32-bit systems.
+
// ScryptAlgorithm is the registration name for the scrypt algorithm instance.
+ // On 64-bit systems this uses scrypt-65536-8-1 (64MB memory).
+ // On 32-bit systems this uses scrypt-16384-4-1 (8MB memory) to avoid OOM.
+ //
+ // For compatibility, both algorithm names are registered and point to the
+ // same implementation with architecture-appropriate parameters.
ScryptAlgorithm = "scrypt-65536-8-1"
+ // Scrypt32BitAlgorithm is an alias for 32-bit compatible scrypt parameters.
+ Scrypt32BitAlgorithm = "scrypt-16384-4-1"
+
// The recommended minimum size for a salt to be used for scrypt.
// Currently set to 16 bytes (128 bits).
//
@@ -21,12 +33,25 @@
)
func init() {
- registerPBKeyDeriver(ScryptAlgorithm, &scryptKeyDeriver{
- n: 65536, //nolint:mnd
- r: 8, //nolint:mnd
- p: 1,
+ // Register the scrypt key deriver with architecture-specific parameters.
+ // We register under both algorithm names for compatibility:
+ // - The standard name "scrypt-65536-8-1" for existing repositories
+ // - The 32-bit name "scrypt-16384-4-1" for 32-bit compatible repositories
+ //
+ // Both names use the same actual parameters (from scryptCostParameter* constants)
+ // which are architecture-dependent.
+ deriver := &scryptKeyDeriver{
+ n: scryptCostParameterN,
+ r: scryptCostParameterR,
+ p: scryptCostParameterP,
minSaltLength: scryptMinSaltLength,
- })
+ }
+
+ // Register under the standard name (used by existing code/repos)
+ registerPBKeyDeriver(ScryptAlgorithm, deriver)
+
+ // Also register under the 32-bit name for clarity
+ registerPBKeyDeriver(Scrypt32BitAlgorithm, deriver)
}
type scryptKeyDeriver struct {
diff --git a/internal/crypto/pb_key_deriver_scrypt_32bit.go b/internal/crypto/pb_key_deriver_scrypt_32bit.go
new file mode 100644
index 00000000..23f27e71
--- /dev/null
+++ b/internal/crypto/pb_key_deriver_scrypt_32bit.go
@@ -0,0 +1,17 @@
+//go:build 386 || arm || mips || mipsle
+
+package crypto
+
+const (
+ // On 32-bit architectures, use reduced scrypt parameters to avoid
+ // running out of address space. The default parameters (N=65536, r=8)
+ // require ~64MB of contiguous memory which can fail on 32-bit systems
+ // with fragmented address space.
+ //
+ // These reduced parameters (N=16384, r=4) require ~8MB of memory,
+ // which is 8x less than the default and provides better compatibility
+ // with 32-bit address space constraints while maintaining reasonable security.
+ scryptCostParameterN = 16384 // reduced from 65536
+ scryptCostParameterR = 4 // reduced from 8
+ scryptCostParameterP = 1 // unchanged
+)
diff --git a/internal/crypto/pb_key_deriver_scrypt_32bit_test.go b/internal/crypto/pb_key_deriver_scrypt_32bit_test.go
new file mode 100644
index 00000000..cde1189f
--- /dev/null
+++ b/internal/crypto/pb_key_deriver_scrypt_32bit_test.go
@@ -0,0 +1,41 @@
+//go:build 386 || arm || mips || mipsle
+
+package crypto
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestScryptParameters32Bit(t *testing.T) {
+ // Verify that 32-bit architectures use reduced scrypt parameters
+ // to avoid running out of address space during key derivation
+ require.Equal(t, 16384, scryptCostParameterN, "32-bit should use N=16384")
+ require.Equal(t, 4, scryptCostParameterR, "32-bit should use r=4")
+ require.Equal(t, 1, scryptCostParameterP, "32-bit should use p=1")
+
+ // Verify both algorithm names are registered
+ require.Equal(t, "scrypt-65536-8-1", ScryptAlgorithm, "ScryptAlgorithm constant should remain unchanged for compatibility")
+ require.Equal(t, "scrypt-16384-4-1", Scrypt32BitAlgorithm, "32-bit algorithm name should be scrypt-16384-4-1")
+
+ // Memory usage: 128 * N * r * p = 128 * 16384 * 4 * 1 = 8,388,608 bytes (~8MB)
+ // This is 8x less than the 64-bit default and should work on 32-bit systems
+ expectedMemory := 128 * scryptCostParameterN * scryptCostParameterR * scryptCostParameterP
+ require.Equal(t, 8388608, expectedMemory, "32-bit should use ~8MB memory for scrypt")
+
+ // Verify that both algorithm names work (they point to the same implementation)
+ salt := make([]byte, 16)
+ password := "test-password"
+
+ key1, err1 := DeriveKeyFromPassword(password, salt, 32, ScryptAlgorithm)
+ require.NoError(t, err1, "Standard scrypt algorithm should be registered and work")
+ require.Len(t, key1, 32, "Should derive 32-byte key")
+
+ key2, err2 := DeriveKeyFromPassword(password, salt, 32, Scrypt32BitAlgorithm)
+ require.NoError(t, err2, "32-bit scrypt algorithm should be registered and work")
+ require.Len(t, key2, 32, "Should derive 32-byte key")
+
+ // Both algorithm names should produce the same result (since they use the same implementation)
+ require.Equal(t, key1, key2, "Both algorithm names should produce identical keys")
+}
diff --git a/internal/crypto/pb_key_deriver_scrypt_64bit.go b/internal/crypto/pb_key_deriver_scrypt_64bit.go
new file mode 100644
index 00000000..99ce422e
--- /dev/null
+++ b/internal/crypto/pb_key_deriver_scrypt_64bit.go
@@ -0,0 +1,12 @@
+//go:build !386 && !arm && !mips && !mipsle
+
+package crypto
+
+const (
+ // Standard scrypt parameters for 64-bit architectures.
+ // These parameters (N=65536, r=8) require ~64MB of memory
+ // and provide strong security guarantees.
+ scryptCostParameterN = 65536
+ scryptCostParameterR = 8
+ scryptCostParameterP = 1
+)
diff --git a/repo/refcount_closer.go b/repo/refcount_closer.go
index b16ced9e..95cb780e 100644
--- a/repo/refcount_closer.go
+++ b/repo/refcount_closer.go
@@ -24,12 +24,13 @@ func (c *refCountedCloser) Close(ctx context.Context) error {
return nil
}
- if c.closed.Load() {
+ // Set closed flag first before checking if already closed
+ // This ensures addRef() racing with us will see the closed flag
+ if !c.closed.CompareAndSwap(false, true) {
+ // Already closed by another goroutine
panic("already closed")
}
- c.closed.Store(true)
-
var errors []error
for _, closer := range c.closers {
@@ -40,11 +41,21 @@ func (c *refCountedCloser) Close(ctx context.Context) error {
}
func (c *refCountedCloser) addRef() {
+ // Increment refcount first
+ newCount := c.refCount.Add(1)
+
+ // Then check if closed - this prevents the race where Close() sets closed=true
+ // after we check but before we increment
if c.closed.Load() {
+ // We were already closed, decrement back and panic
+ c.refCount.Add(-1)
panic("already closed")
}
- c.refCount.Add(1)
+ // Additional safety: if the count went negative, something is very wrong
+ if newCount <= 0 {
+ panic("refcount underflow")
+ }
}
func (c *refCountedCloser) registerEarlyCloseFunc(f closeFunc) {
diff --git a/repo/refcount_closer_test.go b/repo/refcount_closer_test.go
new file mode 100644
index 00000000..93e6629e
--- /dev/null
+++ b/repo/refcount_closer_test.go
@@ -0,0 +1,89 @@
+package repo
+
+import (
+ "context"
+ "sync"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/require"
+)
+
+// TestRefCountedCloserRace tests that the race condition between Close() and addRef() is properly handled
+func TestRefCountedCloserRace(t *testing.T) {
+ t.Parallel()
+
+ // Run the test multiple times to increase chance of catching races
+ for i := 0; i < 100; i++ {
+ t.Run("iteration", func(t *testing.T) {
+ closed := false
+ rcc := newRefCountedCloser(func(ctx context.Context) error {
+ closed = true
+ return nil
+ })
+
+ var wg sync.WaitGroup
+ wg.Add(2)
+
+ addRefSucceeded := false
+
+ // Goroutine 1: Tries to add a ref
+ go func() {
+ defer wg.Done()
+ defer func() {
+ if r := recover(); r != nil {
+ // Expected panic when trying to addRef after close
+ require.Equal(t, "already closed", r)
+ }
+ }()
+ // Small delay to increase likelihood of race
+ time.Sleep(time.Microsecond)
+ rcc.addRef()
+ addRefSucceeded = true
+ }()
+
+ // Goroutine 2: Closes the closer
+ go func() {
+ defer wg.Done()
+ _ = rcc.Close(context.Background())
+ }()
+
+ wg.Wait()
+
+ // If addRef succeeded, we need to close again to trigger the close function
+ if addRefSucceeded {
+ _ = rcc.Close(context.Background())
+ }
+
+ require.True(t, closed, "closer should be closed")
+ })
+ }
+}
+
+// TestRefCountedCloserNormal tests normal operation without races
+func TestRefCountedCloserNormal(t *testing.T) {
+ t.Parallel()
+
+ closeCount := 0
+ rcc := newRefCountedCloser(func(ctx context.Context) error {
+ closeCount++
+ return nil
+ })
+
+ // Add some refs
+ rcc.addRef()
+ rcc.addRef()
+
+ // Close multiple times, but close func should only be called once
+ require.NoError(t, rcc.Close(context.Background()))
+ require.NoError(t, rcc.Close(context.Background()))
+ require.NoError(t, rcc.Close(context.Background()))
+ require.NoError(t, rcc.Close(context.Background()))
+
+ require.Equal(t, 1, closeCount, "close function should be called exactly once")
+
+ // Trying to addRef after close should panic
+ require.Panics(t, func() {
+ rcc.addRef()
+ })
+}
diff --git a/tests/end_to_end_test/api_server_repository_test.go b/tests/end_to_end_test/api_server_repository_test.go
index cc6b458b..e1f82893 100644
--- a/tests/end_to_end_test/api_server_repository_test.go
+++ b/tests/end_to_end_test/api_server_repository_test.go
@@ -269,7 +269,7 @@ func testAPIServerRepository(t *testing.T, allowRepositoryUsers bool) {
}, content.CachingOptions{}, "baz", &repo.Options{})
//nolint:forbidigo
- require.Less(t, timer.Elapsed(), 15*time.Second)
+ require.Less(t, timer.Elapsed(), 30*time.Second)
}
func verifyServerJSONLogs(t require.TestingT, s []string) {