Skip to content

Commit f76edaf

Browse files
committed
Add regression test for semaphore permits after a failed block batch
Drive the real uploadAndCollectBlockData path against a local HTTP test server that fails one block of the batch, with blockUploadSemaphore sized to 2, and assert that both permits can be acquired again afterwards. On master (9d772d0) this fails with "context deadline exceeded" because the collector returns on the first error and the remaining worker stays blocked on the unbuffered result channel, never releasing its permit. With the worker-drain change in this PR it passes, including under -race.
1 parent 1d2e00d commit f76edaf

1 file changed

Lines changed: 106 additions & 31 deletions

File tree

file_upload_concurrency_test.go

Lines changed: 106 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,123 @@
11
package proton_api_bridge
22

33
import (
4+
"bytes"
45
"context"
5-
"errors"
6+
"fmt"
7+
"io"
8+
"net/http"
9+
"net/http/httptest"
610
"testing"
711
"time"
812

13+
"github.com/ProtonMail/gopenpgp/v3/crypto"
14+
"github.com/rclone/go-proton-api"
915
"golang.org/x/sync/semaphore"
1016
)
1117

12-
func TestCollectUploadErrorsReleasesAllWorkersAfterFailure(t *testing.T) {
13-
const (
14-
batchSize = int64(8)
15-
slotCount = int64(20)
16-
)
18+
func TestFailedBlockBatchReleasesEverySemaphorePermit(t *testing.T) {
19+
originalBlockSize := UPLOAD_BLOCK_SIZE
20+
originalBatchSize := UPLOAD_BATCH_BLOCK_SIZE
21+
UPLOAD_BLOCK_SIZE = 16
22+
UPLOAD_BATCH_BLOCK_SIZE = 2
23+
t.Cleanup(func() {
24+
UPLOAD_BLOCK_SIZE = originalBlockSize
25+
UPLOAD_BATCH_BLOCK_SIZE = originalBatchSize
26+
})
1727

18-
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
19-
defer cancel()
20-
slots := semaphore.NewWeighted(slotCount)
21-
22-
for batch := 0; batch < 4; batch++ {
23-
results := make(chan error)
24-
for block := int64(0); block < batchSize; block++ {
25-
go func(fail bool) {
26-
if err := slots.Acquire(ctx, 1); err != nil {
27-
results <- err
28-
return
29-
}
30-
defer slots.Release(1)
31-
if fail {
32-
results <- errors.New("synthetic upload failure")
33-
return
34-
}
35-
results <- nil
36-
}(block == 0)
37-
}
28+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
29+
w.Header().Set("Date", time.Now().UTC().Format(http.TimeFormat))
30+
w.Header().Set("Content-Type", "application/json")
3831

39-
if err := collectUploadErrors(results, int(batchSize)); err == nil {
40-
t.Fatal("expected the first upload failure to be returned")
32+
switch r.URL.Path {
33+
case "/drive/blocks":
34+
_, err := io.WriteString(w, fmt.Sprintf(
35+
`{"Code":1000,"UploadLinks":[{"Token":"error-token","BareURL":%q},{"Token":"success-token","BareURL":%q}]}`,
36+
serverURL(r, "/storage/blocks/error"),
37+
serverURL(r, "/storage/blocks/success"),
38+
))
39+
if err != nil {
40+
t.Error(err)
41+
}
42+
case "/storage/blocks/error":
43+
w.WriteHeader(http.StatusBadGateway)
44+
_, err := io.WriteString(w, `{"Code":0,"Error":"simulated bad gateway"}`)
45+
if err != nil {
46+
t.Error(err)
47+
}
48+
case "/storage/blocks/success":
49+
time.Sleep(100 * time.Millisecond)
50+
_, err := io.WriteString(w, `{"Code":1000}`)
51+
if err != nil {
52+
t.Error(err)
53+
}
54+
default:
55+
http.NotFound(w, r)
4156
}
57+
}))
58+
defer server.Close()
59+
60+
manager := proton.New(
61+
proton.WithHostURL(server.URL),
62+
proton.WithRetryCount(0),
63+
)
64+
defer manager.Close()
65+
client := manager.NewClient("", "", "")
66+
defer client.Close()
67+
68+
pgp := crypto.PGP()
69+
signingKey, err := pgp.KeyGeneration().AddUserId("test", "test@example.com").New().GenerateKey()
70+
if err != nil {
71+
t.Fatal(err)
72+
}
73+
signingKeyRing, err := crypto.NewKeyRing(signingKey)
74+
if err != nil {
75+
t.Fatal(err)
76+
}
77+
nodeKey, err := pgp.KeyGeneration().AddUserId("node", "node@example.com").New().GenerateKey()
78+
if err != nil {
79+
t.Fatal(err)
80+
}
81+
nodeKeyRing, err := crypto.NewKeyRing(nodeKey)
82+
if err != nil {
83+
t.Fatal(err)
84+
}
85+
sessionKey, err := pgp.GenerateSessionKey()
86+
if err != nil {
87+
t.Fatal(err)
4288
}
4389

44-
if err := slots.Acquire(ctx, slotCount); err != nil {
45-
t.Fatalf("upload workers leaked semaphore slots: %v", err)
90+
blockSemaphore := semaphore.NewWeighted(2)
91+
drive := &ProtonDrive{
92+
MainShare: &proton.Share{
93+
ShareMetadata: proton.ShareMetadata{ShareID: "share-id"},
94+
AddressID: "address-id",
95+
},
96+
DefaultAddrKR: signingKeyRing,
97+
c: client,
98+
blockUploadSemaphore: blockSemaphore,
4699
}
47-
slots.Release(slotCount)
100+
101+
_, _, _, _, err = drive.uploadAndCollectBlockData(
102+
context.Background(),
103+
sessionKey,
104+
nodeKeyRing,
105+
bytes.NewReader([]byte("0123456789abcdef0123456789abcdef")),
106+
"link-id",
107+
"revision-id",
108+
)
109+
if err == nil {
110+
t.Fatal("expected failed block upload")
111+
}
112+
113+
acquireCtx, cancel := context.WithTimeout(context.Background(), time.Second)
114+
defer cancel()
115+
if err := blockSemaphore.Acquire(acquireCtx, 2); err != nil {
116+
t.Fatalf("failed batch leaked a block-upload permit: %v", err)
117+
}
118+
blockSemaphore.Release(2)
119+
}
120+
121+
func serverURL(r *http.Request, path string) string {
122+
return "http://" + r.Host + path
48123
}

0 commit comments

Comments
 (0)