| 1 | package idear_test |
| 2 | |
| 3 | import ( |
| 4 | "errors" |
| 5 | "fmt" |
| 6 | "sync" |
| 7 | "testing" |
| 8 | |
| 9 | "amadan.net/rastrillo/idear" |
| 10 | "amadan.net/rastrillo/idear/internal/ideartest" |
| 11 | ) |
| 12 | |
| 13 | // These four are the point of the store, and they are written as |
| 14 | // ACTUAL races: goroutines released together off a start barrier, not |
| 15 | // sequential calls arranged to look concurrent. Round 1 of the CARLOS |
| 16 | // bake-off found a real ownership-transfer defect in a hand-rolled |
| 17 | // membership layer, and found it only because someone ran it as a |
| 18 | // race; a sequential rehearsal of the same calls passes over the bug, |
| 19 | // because the bug lives in the window between a read and a write and a |
| 20 | // sequential test never opens that window. |
| 21 | // |
| 22 | // Every worker below drives h.Roster directly and returns its error |
| 23 | // through a slice. None of them touch testing.T, and none call a |
| 24 | // harness helper: those report with t.Fatalf, which is only legal on |
| 25 | // the test goroutine. |
| 26 | // |
| 27 | // A note on what these can and cannot prove. rastrillo/db's writer |
| 28 | // pool holds exactly ONE connection, so write transactions queue in |
| 29 | // database/sql rather than collide in SQLite — which is precisely why |
| 30 | // none of these need a retry loop or a sleep, and why seeing |
| 31 | // "database is locked" here would be a bug in the store (a |
| 32 | // transaction held open across something that is not a database |
| 33 | // operation) and never something to paper over. What the single writer |
| 34 | // does NOT do is serialise a read-then-write pair that spans two |
| 35 | // transactions. That gap is the whole attack surface, and it is what |
| 36 | // these tests aim at. |
| 37 | |
| 38 | // release returns a start barrier: the workers block on wait() until |
| 39 | // the test closes the gate, so they enter the store together instead |
| 40 | // of trickling in as they are spawned. |
| 41 | func release() (gate chan struct{}, wait func()) { |
| 42 | gate = make(chan struct{}) |
| 43 | return gate, func() { <-gate } |
| 44 | } |
| 45 | |
| 46 | // pair runs two operations concurrently off one start barrier, with |
| 47 | // the spawn order flipped when swap is true. |
| 48 | // |
| 49 | // The flip is not decoration. Closing a channel wakes its waiters in |
| 50 | // FIFO order and the LAST one readied lands in the P's runnext slot, |
| 51 | // so it is the last-spawned goroutine that actually runs first — a |
| 52 | // systematic bias, and a measured one: without the flip, Revoke won |
| 53 | // 58-60 of every 60 rounds below and the Accept-wins branch of the |
| 54 | // invariant was barely exercised at all. A race test that only ever |
| 55 | // resolves one way is green for a reason unrelated to the property it |
| 56 | // claims to check. Alternating the spawn order splits the outcomes |
| 57 | // without weakening the race: both goroutines are still released |
| 58 | // together and still contend for the same single writer connection. |
| 59 | func pair(swap bool, first, second func()) { |
| 60 | if swap { |
| 61 | first, second = second, first |
| 62 | } |
| 63 | gate, wait := release() |
| 64 | var wg sync.WaitGroup |
| 65 | wg.Add(2) |
| 66 | go func() { |
| 67 | defer wg.Done() |
| 68 | wait() |
| 69 | first() |
| 70 | }() |
| 71 | go func() { |
| 72 | defer wg.Done() |
| 73 | wait() |
| 74 | second() |
| 75 | }() |
| 76 | close(gate) |
| 77 | wg.Wait() |
| 78 | } |
| 79 | |
| 80 | // TestConcurrentClaimYieldsOneOwner races six first-signups at an |
| 81 | // unclaimed instance. |
| 82 | // |
| 83 | // Regression it catches: Claim counting rows OUTSIDE its transaction, |
| 84 | // or counting only ACTIVE rows. Either way more than one goroutine |
| 85 | // sees an empty roster, and the instance ends up with two Owners — |
| 86 | // two people who can each demote the other, on a roster that is |
| 87 | // supposed to have exactly one root of authority. |
| 88 | func TestConcurrentClaimYieldsOneOwner(t *testing.T) { |
| 89 | // Six racers, on a FRESH instance, repeated. A single round of six |
| 90 | // was measured at only ~84% detection (21 failures in 25 runs) |
| 91 | // against the mutation that moves the count out of the |
| 92 | // transaction — so a one-shot `go test` missed the single most |
| 93 | // important regression in this file about one run in five. Rounds |
| 94 | // are independent trials: twenty of them put a miss past 1 in |
| 95 | // 10^17, which is the difference between a test and a coin. |
| 96 | const ( |
| 97 | n = 6 |
| 98 | rounds = 20 |
| 99 | ) |
| 100 | |
| 101 | for round := range rounds { |
| 102 | h := ideartest.New(t) |
| 103 | ctx := h.Ctx() |
| 104 | |
| 105 | gate, wait := release() |
| 106 | errs := make([]error, n) |
| 107 | var wg sync.WaitGroup |
| 108 | for i := range n { |
| 109 | wg.Add(1) |
| 110 | go func() { |
| 111 | defer wg.Done() |
| 112 | wait() |
| 113 | _, errs[i] = h.Roster.Claim(ctx, |
| 114 | fmt.Sprintf("claimant-%d", i), |
| 115 | fmt.Sprintf("claimant-%d@example.test", i), |
| 116 | fmt.Sprintf("Claimant %d", i)) |
| 117 | }() |
| 118 | } |
| 119 | close(gate) |
| 120 | wg.Wait() |
| 121 | |
| 122 | won, refused := 0, 0 |
| 123 | for i, err := range errs { |
| 124 | switch { |
| 125 | case err == nil: |
| 126 | won++ |
| 127 | case errors.Is(err, idear.ErrOwnerExists): |
| 128 | refused++ |
| 129 | default: |
| 130 | t.Fatalf("round %d: claimant %d failed with an unexpected error: %v", round, i, err) |
| 131 | } |
| 132 | } |
| 133 | if won != 1 { |
| 134 | t.Fatalf("round %d: %d of %d concurrent claims succeeded, want exactly 1", round, won, n) |
| 135 | } |
| 136 | if refused != n-1 { |
| 137 | t.Fatalf("round %d: %d claims were refused with ErrOwnerExists, want %d", round, refused, n-1) |
| 138 | } |
| 139 | if got := h.CountMembers(); got != 1 { |
| 140 | t.Fatalf("round %d: the roster holds %d rows, want exactly 1", round, got) |
| 141 | } |
| 142 | if owner := h.TheOwner(); !owner.Active() { |
| 143 | t.Fatalf("round %d: the owner is not active", round) |
| 144 | } |
| 145 | } |
| 146 | } |
| 147 | |
| 148 | // TestConcurrentTransfersKeepOneOwner races six transfers out of the |
| 149 | // same Owner, each to a different target. |
| 150 | // |
| 151 | // Regression it catches: Transfer trusting the *Member it was handed |
| 152 | // instead of re-reading the actor's row inside its transaction. Every |
| 153 | // goroutine holds a struct that says "I am the Owner", and every one |
| 154 | // of them is telling the truth about the moment it was read. Without |
| 155 | // the re-read, all six promote their target and the instance ends up |
| 156 | // with six Owners and no way back — this is the exact defect round 1 |
| 157 | // of the bake-off found in a hand-rolled version. |
| 158 | func TestConcurrentTransfersKeepOneOwner(t *testing.T) { |
| 159 | const n = 6 |
| 160 | h := ideartest.New(t) |
| 161 | ctx := h.Ctx() |
| 162 | |
| 163 | owner := h.Owner() |
| 164 | targets := make([]*idear.Member, n) |
| 165 | for i := range targets { |
| 166 | targets[i] = h.Member(idear.RoleAdmin) |
| 167 | } |
| 168 | |
| 169 | gate, wait := release() |
| 170 | errs := make([]error, n) |
| 171 | var wg sync.WaitGroup |
| 172 | for i := range n { |
| 173 | wg.Add(1) |
| 174 | go func() { |
| 175 | defer wg.Done() |
| 176 | wait() |
| 177 | errs[i] = h.Roster.Transfer(ctx, owner, targets[i]) |
| 178 | }() |
| 179 | } |
| 180 | close(gate) |
| 181 | wg.Wait() |
| 182 | |
| 183 | won := 0 |
| 184 | for i, err := range errs { |
| 185 | switch { |
| 186 | case err == nil: |
| 187 | won++ |
| 188 | case errors.Is(err, idear.ErrForbidden): |
| 189 | // The transaction re-read an actor who is no longer Owner. |
| 190 | default: |
| 191 | t.Errorf("transfer %d failed with an unexpected error: %v", i, err) |
| 192 | } |
| 193 | } |
| 194 | if won != 1 { |
| 195 | t.Errorf("%d of %d concurrent transfers succeeded, want exactly 1", won, n) |
| 196 | } |
| 197 | |
| 198 | // The count is the invariant, not the number of successes: a |
| 199 | // transaction that half-committed would leave two Owners while |
| 200 | // reporting one success. |
| 201 | newOwner := h.TheOwner() |
| 202 | if !newOwner.Active() { |
| 203 | t.Error("the new owner is not active") |
| 204 | } |
| 205 | if newOwner.ID == owner.ID { |
| 206 | t.Error("ownership did not move at all; one transfer should have won") |
| 207 | } |
| 208 | if got := h.Reload(owner.ID).Role; got != idear.RoleAdmin { |
| 209 | t.Errorf("the outgoing owner's role = %q, want admin — the demote and the promote are one transaction", got) |
| 210 | } |
| 211 | } |
| 212 | |
| 213 | // TestRevokeRacingAcceptNeverAdmits races a Revoke against an Accept of |
| 214 | // the same invitation, many times over, each iteration on a fresh |
| 215 | // invitation. |
| 216 | // |
| 217 | // Regression it catches: Accept consuming the invitation by lookup and |
| 218 | // then writing, instead of by compare-and-swap with rows-affected |
| 219 | // checked. Between the lookup and the write, Revoke commits — and the |
| 220 | // invitation is admitted after it was withdrawn, which is the whole |
| 221 | // point of being able to withdraw one. Softening the CAS's WHERE |
| 222 | // clause (dropping "revoked_at IS NULL", say) fails here too, as does |
| 223 | // moving the Member insert out of the invitation's transaction. |
| 224 | // |
| 225 | // Note that "never both" is the assertion, not "Accept always loses". |
| 226 | // Either outcome is correct; what is not correct is both succeeding. |
| 227 | func TestRevokeRacingAcceptNeverAdmits(t *testing.T) { |
| 228 | const iterations = 60 |
| 229 | h := ideartest.New(t) |
| 230 | ctx := h.Ctx() |
| 231 | owner := h.Owner() |
| 232 | |
| 233 | admitted, revoked, both, neither := 0, 0, 0, 0 |
| 234 | for i := range iterations { |
| 235 | email := fmt.Sprintf("racer-%d@example.test", i) |
| 236 | subject := fmt.Sprintf("racer-%d", i) |
| 237 | inv, token, err := h.Roster.Invite(ctx, owner, email, idear.RoleMember) |
| 238 | if err != nil { |
| 239 | t.Fatalf("iteration %d: Invite: %v", i, err) |
| 240 | } |
| 241 | |
| 242 | var ( |
| 243 | acceptErr, revokeErr error |
| 244 | acceptedM *idear.Member |
| 245 | ) |
| 246 | pair(i%2 == 1, |
| 247 | func() { acceptedM, acceptErr = h.Roster.Accept(ctx, token, subject, "Racer") }, |
| 248 | func() { revokeErr = h.Roster.Revoke(ctx, owner, inv.ID) }, |
| 249 | ) |
| 250 | acceptDone, revokeDone := acceptErr == nil, revokeErr == nil |
| 251 | |
| 252 | if acceptErr != nil && !errors.Is(acceptErr, idear.ErrNoInvitation) { |
| 253 | t.Fatalf("iteration %d: Accept failed unexpectedly: %v", i, acceptErr) |
| 254 | } |
| 255 | if revokeErr != nil && !errors.Is(revokeErr, idear.ErrNoInvitation) { |
| 256 | t.Fatalf("iteration %d: Revoke failed unexpectedly: %v", i, revokeErr) |
| 257 | } |
| 258 | |
| 259 | switch { |
| 260 | case acceptDone && revokeDone: |
| 261 | both++ |
| 262 | t.Errorf("iteration %d: the invitation was BOTH accepted and revoked", i) |
| 263 | case acceptDone: |
| 264 | admitted++ |
| 265 | case revokeDone: |
| 266 | revoked++ |
| 267 | default: |
| 268 | neither++ |
| 269 | t.Errorf("iteration %d: neither Accept nor Revoke succeeded (accept=%v revoke=%v)", i, acceptErr, revokeErr) |
| 270 | } |
| 271 | |
| 272 | // The row's own state must agree with who won, and a member |
| 273 | // must exist if and only if Accept won. A CAS that updated the |
| 274 | // invitation but lost the member insert would show up here. |
| 275 | stored := h.Invitation(inv.ID) |
| 276 | _, lookupErr := h.Roster.BySubject(ctx, subject) |
| 277 | if acceptDone { |
| 278 | if stored.AcceptedAt == nil { |
| 279 | t.Errorf("iteration %d: Accept succeeded but AcceptedAt is NULL", i) |
| 280 | } |
| 281 | if stored.RevokedAt != nil { |
| 282 | t.Errorf("iteration %d: the accepted invitation is also marked revoked", i) |
| 283 | } |
| 284 | if lookupErr != nil { |
| 285 | t.Errorf("iteration %d: Accept succeeded but the member is not in the roster: %v", i, lookupErr) |
| 286 | } |
| 287 | if acceptedM != nil && acceptedM.Role != idear.RoleMember { |
| 288 | t.Errorf("iteration %d: admitted at %q, want the invited role", i, acceptedM.Role) |
| 289 | } |
| 290 | } else { |
| 291 | if stored.AcceptedAt != nil { |
| 292 | t.Errorf("iteration %d: Accept failed but the invitation is marked accepted", i) |
| 293 | } |
| 294 | if !errors.Is(lookupErr, idear.ErrNotFound) { |
| 295 | t.Errorf("iteration %d: a refused Accept admitted the subject anyway: %v", i, lookupErr) |
| 296 | } |
| 297 | } |
| 298 | } |
| 299 | |
| 300 | t.Logf("%d iterations: %d admitted, %d revoked, %d both, %d neither", |
| 301 | iterations, admitted, revoked, both, neither) |
| 302 | // Both outcomes should actually occur across this many rounds. If |
| 303 | // one never does, the race is not being exercised — the test would |
| 304 | // still be green while proving nothing. |
| 305 | if admitted == 0 || revoked == 0 { |
| 306 | t.Errorf("only one outcome ever occurred (%d admitted, %d revoked); "+ |
| 307 | "the two calls are not actually racing and this test proves nothing", |
| 308 | admitted, revoked) |
| 309 | } |
| 310 | } |
| 311 | |
| 312 | // TestTransferRacingDeactivateNeverStrandsOwner races a Transfer of |
| 313 | // ownership TO a member against a Deactivate OF that same member. |
| 314 | // |
| 315 | // Regression it catches: Transfer not confirming, inside its own |
| 316 | // transaction, that the target is still active. The losing order is |
| 317 | // Deactivate-then-Transfer: the transfer promotes a row that was |
| 318 | // deactivated a microsecond ago, and the instance now has a |
| 319 | // DEACTIVATED Owner. That is terminal through idear's own API — |
| 320 | // MayActOn refuses acting on an Owner at every rank, so nobody can |
| 321 | // reactivate them, nobody can demote them, and nobody can be promoted |
| 322 | // past them. It also catches Deactivate checking "is this the Owner" |
| 323 | // against the caller's stale struct rather than the row: in the other |
| 324 | // order, the target IS the Owner by the time Deactivate's transaction |
| 325 | // runs, and must be refused. |
| 326 | func TestTransferRacingDeactivateNeverStrandsOwner(t *testing.T) { |
| 327 | const iterations = 40 |
| 328 | transferWins, deactivateWins, bothFailed := 0, 0, 0 |
| 329 | |
| 330 | for i := range iterations { |
| 331 | h := ideartest.New(t) |
| 332 | ctx := h.Ctx() |
| 333 | owner := h.Owner() |
| 334 | target := h.Member(idear.RoleAdmin) |
| 335 | |
| 336 | var transferErr, removeErr error |
| 337 | pair(i%2 == 1, |
| 338 | func() { transferErr = h.Roster.Transfer(ctx, owner, target) }, |
| 339 | func() { removeErr = h.Roster.Deactivate(ctx, owner, target) }, |
| 340 | ) |
| 341 | switch { |
| 342 | case transferErr == nil: |
| 343 | transferWins++ |
| 344 | case removeErr == nil: |
| 345 | deactivateWins++ |
| 346 | default: |
| 347 | // Both refused. Folding this into "deactivate won" would |
| 348 | // let the both-outcomes-occurred check below pass on a |
| 349 | // run where neither operation ever succeeded. |
| 350 | bothFailed++ |
| 351 | } |
| 352 | |
| 353 | if transferErr != nil && !errors.Is(transferErr, idear.ErrForbidden) { |
| 354 | t.Fatalf("iteration %d: Transfer failed unexpectedly: %v", i, transferErr) |
| 355 | } |
| 356 | if removeErr != nil && !errors.Is(removeErr, idear.ErrForbidden) && !errors.Is(removeErr, idear.ErrLastOwner) { |
| 357 | t.Fatalf("iteration %d: Deactivate failed unexpectedly: %v", i, removeErr) |
| 358 | } |
| 359 | |
| 360 | // The invariant, whichever order won: exactly one Owner, and |
| 361 | // that Owner is ACTIVE. |
| 362 | current := h.TheOwner() |
| 363 | if !current.Active() { |
| 364 | t.Fatalf("iteration %d: the instance has a DEACTIVATED owner (member %d) — "+ |
| 365 | "nobody can administer it and nobody can be promoted (transfer=%v deactivate=%v)", |
| 366 | i, current.ID, transferErr, removeErr) |
| 367 | } |
| 368 | |
| 369 | switch { |
| 370 | case transferErr == nil: |
| 371 | // Transfer won the race, so Deactivate must have been |
| 372 | // refused: by the time it ran, its target was the Owner. |
| 373 | if current.ID != target.ID { |
| 374 | t.Errorf("iteration %d: Transfer succeeded but the owner is %d, not %d", i, current.ID, target.ID) |
| 375 | } |
| 376 | if removeErr == nil { |
| 377 | t.Errorf("iteration %d: the new owner was deactivated by the racing Deactivate", i) |
| 378 | } |
| 379 | default: |
| 380 | // Deactivate won, so ownership must not have moved. |
| 381 | if current.ID != owner.ID { |
| 382 | t.Errorf("iteration %d: Transfer was refused but ownership moved to %d", i, current.ID) |
| 383 | } |
| 384 | } |
| 385 | } |
| 386 | |
| 387 | t.Logf("%d iterations: transfer won %d, deactivate won %d, both refused %d", |
| 388 | iterations, transferWins, deactivateWins, bothFailed) |
| 389 | if transferWins == 0 || deactivateWins == 0 { |
| 390 | t.Errorf("only one order ever occurred (transfer %d, deactivate %d, both refused %d); "+ |
| 391 | "the two calls are not actually racing and this test proves nothing", |
| 392 | transferWins, deactivateWins, bothFailed) |
| 393 | } |
| 394 | if bothFailed != 0 { |
| 395 | t.Errorf("%d iterations refused BOTH operations; one of them must always be able to win", |
| 396 | bothFailed) |
| 397 | } |
| 398 | } |
| 399 | |
amadan