diff --git a/.changeset/fix-pubsub-replay-retention.md b/.changeset/fix-pubsub-replay-retention.md new file mode 100644 index 00000000000..632765fdcf7 --- /dev/null +++ b/.changeset/fix-pubsub-replay-retention.md @@ -0,0 +1,5 @@ +--- +"effect": patch +--- + +Prevent replay-enabled PubSubs from retaining values beyond each subscription's replay window. diff --git a/packages/effect/src/PubSub.ts b/packages/effect/src/PubSub.ts index c7f0d62b133..dc9ce053179 100644 --- a/packages/effect/src/PubSub.ts +++ b/packages/effect/src/PubSub.ts @@ -139,6 +139,7 @@ export declare namespace PubSub { take(): A | undefined takeN(n: number): Array takeAll(): Array + close(): void readonly remaining: number } @@ -1115,6 +1116,7 @@ const unsubscribe = (self: Subscription): Effect.Effect => Effect.sync(() => { self.subscribers.delete(self.subscription) self.subscription.unsubscribe() + self.replayWindow.close() self.strategy.onPubSubEmptySpaceUnsafe(self.pubsub, self.subscribers) }) ), @@ -1526,6 +1528,7 @@ const makeSubscriptionUnsafe = ( class BoundedPubSubArb implements PubSub.Atomic { array: Array + replayIndices: Array publisherIndex = 0 subscribers: Array subscriberCount = 0 @@ -1538,6 +1541,7 @@ class BoundedPubSubArb implements PubSub.Atomic { this.capacity = capacity this.replayBuffer = replayBuffer this.array = Array.from({ length: capacity }) + this.replayIndices = replayBuffer ? Array.from({ length: capacity }) : [] this.subscribers = Array.from({ length: capacity }) } @@ -1561,15 +1565,16 @@ class BoundedPubSubArb implements PubSub.Atomic { if (this.isFull()) { return false } + const replayIndex = this.replayBuffer?.offer(value) if (this.subscriberCount !== 0) { const index = this.publisherIndex % this.capacity this.array[index] = value + if (replayIndex !== undefined) { + this.replayIndices[index] = replayIndex + } this.subscribers[index] = this.subscriberCount this.publisherIndex += 1 } - if (this.replayBuffer) { - this.replayBuffer.offer(value) - } return true } @@ -1594,11 +1599,12 @@ class BoundedPubSubArb implements PubSub.Atomic { const a = chunk[iteratorIndex++] const index = this.publisherIndex % this.capacity this.array[index] = a + const replayIndex = this.replayBuffer?.offer(a) + if (replayIndex !== undefined) { + this.replayIndices[index] = replayIndex + } this.subscribers[index] = this.subscriberCount this.publisherIndex += 1 - if (this.replayBuffer) { - this.replayBuffer.offer(a) - } } return chunk.slice(iteratorIndex) } @@ -1606,12 +1612,11 @@ class BoundedPubSubArb implements PubSub.Atomic { slide(): void { if (this.subscribersIndex !== this.publisherIndex) { const index = this.subscribersIndex % this.capacity + const value = this.array[index] this.array[index] = AbsentValue as unknown as A this.subscribers[index] = 0 this.subscribersIndex += 1 - } - if (this.replayBuffer) { - this.replayBuffer.slide() + this.replayBuffer?.slide(value, this.replayIndices[index]) } } @@ -1717,6 +1722,7 @@ class BoundedPubSubArbSubscription implements PubSub.BackingSubscripti class BoundedPubSubPow2 implements PubSub.Atomic { array: Array + replayIndices: Array mask: number publisherIndex = 0 subscribers: Array @@ -1730,6 +1736,7 @@ class BoundedPubSubPow2 implements PubSub.Atomic { this.capacity = capacity this.replayBuffer = replayBuffer this.array = Array.from({ length: capacity }) + this.replayIndices = replayBuffer ? Array.from({ length: capacity }) : [] this.mask = capacity - 1 this.subscribers = Array.from({ length: capacity }) } @@ -1754,15 +1761,16 @@ class BoundedPubSubPow2 implements PubSub.Atomic { if (this.isFull()) { return false } + const replayIndex = this.replayBuffer?.offer(value) if (this.subscriberCount !== 0) { const index = this.publisherIndex & this.mask this.array[index] = value + if (replayIndex !== undefined) { + this.replayIndices[index] = replayIndex + } this.subscribers[index] = this.subscriberCount this.publisherIndex += 1 } - if (this.replayBuffer) { - this.replayBuffer.offer(value) - } return true } @@ -1787,11 +1795,12 @@ class BoundedPubSubPow2 implements PubSub.Atomic { const elem = chunk[iteratorIndex++] const index = this.publisherIndex & this.mask this.array[index] = elem + const replayIndex = this.replayBuffer?.offer(elem) + if (replayIndex !== undefined) { + this.replayIndices[index] = replayIndex + } this.subscribers[index] = this.subscriberCount this.publisherIndex += 1 - if (this.replayBuffer) { - this.replayBuffer.offer(elem) - } } return chunk.slice(iteratorIndex) } @@ -1799,12 +1808,11 @@ class BoundedPubSubPow2 implements PubSub.Atomic { slide(): void { if (this.subscribersIndex !== this.publisherIndex) { const index = this.subscribersIndex & this.mask + const value = this.array[index] this.array[index] = AbsentValue as unknown as A this.subscribers[index] = 0 this.subscribersIndex += 1 - } - if (this.replayBuffer) { - this.replayBuffer.slide() + this.replayBuffer?.slide(value, this.replayIndices[index]) } } @@ -1912,6 +1920,7 @@ class BoundedPubSubSingle implements PubSub.Atomic { subscriberCount = 0 subscribers = 0 value: A = AbsentValue as unknown as A + replayIndex = 0 readonly capacity = 1 readonly replayBuffer: ReplayBuffer | undefined @@ -1944,14 +1953,15 @@ class BoundedPubSubSingle implements PubSub.Atomic { if (this.isFull()) { return false } + const replayIndex = this.replayBuffer?.offer(value) if (this.subscriberCount !== 0) { this.value = value + if (replayIndex !== undefined) { + this.replayIndex = replayIndex + } this.subscribers = this.subscriberCount this.publisherIndex += 1 } - if (this.replayBuffer) { - this.replayBuffer.offer(value) - } return true } @@ -1975,11 +1985,10 @@ class BoundedPubSubSingle implements PubSub.Atomic { slide(): void { if (this.isFull()) { + const value = this.value this.subscribers = 0 this.value = AbsentValue as unknown as A - } - if (this.replayBuffer) { - this.replayBuffer.slide() + this.replayBuffer?.slide(value, this.replayIndex) } } @@ -2058,6 +2067,7 @@ class BoundedPubSubSingleSubscription implements PubSub.BackingSubscri interface Node { value: A | AbsentValue + replayIndex: number | undefined subscribers: number next: Node | null } @@ -2065,6 +2075,7 @@ interface Node { class UnboundedPubSub implements PubSub.Atomic { publisherHead: Node = { value: AbsentValue, + replayIndex: undefined, subscribers: 0, next: null } @@ -2096,19 +2107,19 @@ class UnboundedPubSub implements PubSub.Atomic { } publish(value: A): boolean { + const replayIndex = this.replayBuffer?.offer(value) const subscribers = this.publisherTail.subscribers if (subscribers !== 0) { - this.publisherTail.next = { + const node: Node = { value, + replayIndex, subscribers, next: null } + this.publisherTail.next = node this.publisherTail = this.publisherTail.next this.publisherIndex += 1 } - if (this.replayBuffer) { - this.replayBuffer.offer(value) - } return true } @@ -2125,12 +2136,12 @@ class UnboundedPubSub implements PubSub.Atomic { slide(): void { if (this.publisherHead !== this.publisherTail) { + const node = this.publisherHead.next! + const value = node.value as A this.publisherHead = this.publisherHead.next! this.publisherHead.value = AbsentValue this.subscribersIndex += 1 - } - if (this.replayBuffer) { - this.replayBuffer.slide() + this.replayBuffer?.slide(value, node.replayIndex!) } } @@ -2702,27 +2713,40 @@ const strategyCompleteSubscribersUnsafe = ( interface ReplayNode { value: A | AbsentValue + index: number next: ReplayNode | null } class ReplayBuffer { readonly capacity: number - head: ReplayNode = { value: AbsentValue, next: null } + head: ReplayNode = { value: AbsentValue, index: 0, next: null } tail: ReplayNode = this.head + readonly slideValues: Array<{ + readonly value: A + readonly index: number + }> = [] size = 0 index = 0 + publisherIndex = 0 constructor(capacity: number) { this.capacity = capacity } - slide() { + slide(value: A, publisherIndex: number): void { + this.slideValues[this.index % this.capacity] = { + value, + index: publisherIndex + } this.index++ } - offer(a: A): void { + offer(a: A): number { + const index = this.publisherIndex++ this.tail.value = a + this.tail.index = index this.tail.next = { value: AbsentValue, + index: 0, next: null } this.tail = this.tail.next @@ -2731,6 +2755,7 @@ class ReplayBuffer { } else { this.size += 1 } + return index } offerAll(as: Iterable): void { for (const a of as) { @@ -2740,48 +2765,66 @@ class ReplayBuffer { } class ReplayWindowImpl implements PubSub.ReplayWindow { - head: ReplayNode - index: number - remaining: number readonly buffer: ReplayBuffer + readonly values: Array + index = 0 + remaining: number + slideIndex: number + newestIndex = -1 constructor(buffer: ReplayBuffer) { this.buffer = buffer - this.index = buffer.index this.remaining = buffer.size - this.head = buffer.head - } - fastForward() { - while (this.index < this.buffer.index) { - this.head = this.head.next! - this.index++ + this.slideIndex = buffer.index + this.values = new Array(this.remaining) + let node = buffer.head + for (let i = 0; i < this.remaining; i++) { + this.values[i] = node.value as A + this.newestIndex = node.index + node = node.next! + } + } + close(): void { + this.values.length = 0 + this.remaining = 0 + } + sync(): void { + const slides = this.buffer.index - this.slideIndex + if (slides === 0 || this.remaining === 0) { + return + } + const count = Math.min(slides, this.buffer.capacity) + const start = this.buffer.index - count + for (let i = 0; i < count; i++) { + const entry = this.buffer.slideValues[(start + i) % this.buffer.capacity] + if (entry.index > this.newestIndex) { + this.index = (this.index + 1) % this.values.length + this.values[(this.index + this.remaining - 1) % this.values.length] = entry.value + this.newestIndex = entry.index + } } + this.slideIndex = this.buffer.index } take(): A | undefined { if (this.remaining === 0) { return undefined - } else if (this.index < this.buffer.index) { - this.fastForward() } + this.sync() + const value = this.values[this.index] + this.values[this.index] = AbsentValue as unknown as A + this.index = (this.index + 1) % this.values.length this.remaining-- - const value = this.head.value - this.head = this.head.next! + if (this.remaining === 0) { + this.close() + } return value as A } takeN(n: number): Array { - if (this.remaining === 0) { - return [] - } else if (this.index < this.buffer.index) { - this.fastForward() - } const len = Math.min(n, this.remaining) const items = new Array(len) for (let i = 0; i < len; i++) { - const value = this.head.value as A - this.head = this.head.next! - items[i] = value + items[i] = this.take()! } - this.remaining -= len return items } takeAll(): Array { @@ -2793,5 +2836,6 @@ const emptyReplayWindow: PubSub.ReplayWindow = { remaining: 0, take: () => undefined, takeN: () => [], - takeAll: () => [] + takeAll: () => [], + close: () => void 0 } diff --git a/packages/effect/test/PubSub.test.ts b/packages/effect/test/PubSub.test.ts index bf1b79172d0..02972b61173 100644 --- a/packages/effect/test/PubSub.test.ts +++ b/packages/effect/test/PubSub.test.ts @@ -399,6 +399,48 @@ describe("PubSub", () => { })) describe("replay", () => { + it("does not retain values published after the replay window is drained", () => { + const pubsub = PubSub.makeAtomicUnbounded({ replay: 1 }) + pubsub.publish({}) + const replayWindow = pubsub.replayWindow() + replayWindow.take() + + const slidOut = {} + pubsub.publish(slidOut) + pubsub.publish({}) + + assert.isFalse(retains(replayWindow, slidOut)) + }) + + it("does not retain values published outside an undrained replay window", () => { + const pubsub = PubSub.makeAtomicUnbounded({ replay: 1 }) + const replayed = {} + pubsub.publish(replayed) + const replayWindow = pubsub.replayWindow() + + const slidOut = {} + pubsub.publish(slidOut) + pubsub.publish({}) + + assert.isFalse(retains(replayWindow, slidOut)) + assert.strictEqual(replayWindow.take(), replayed) + }) + + it("preserves replay order across multiple slides", () => { + const pubsub = PubSub.makeAtomicBounded({ capacity: 4, replay: 3 }) + pubsub.publishAll([1, 2, 3, 4, 5]) + const subscription = pubsub.subscribe() + const replayWindow = pubsub.replayWindow() + pubsub.publishAll([6, 7, 8, 9]) + for (const value of [10, 11, 12]) { + pubsub.slide() + pubsub.publish(value) + } + + assert.deepStrictEqual(replayWindow.takeAll(), [6, 7, 8]) + assert.deepStrictEqual(subscription.pollUpTo(Number.POSITIVE_INFINITY), [9, 10, 11, 12]) + }) + it.effect("unbounded", () => Effect.gen(function*() { const messages = [1, 2, 3, 4, 5] @@ -474,6 +516,20 @@ describe("PubSub", () => { const sub3 = yield* PubSub.subscribe(pubsub) assert.deepStrictEqual(yield* PubSub.takeAll(sub3), [14, 15, 16]) })) + + it.effect("sliding preserves publish order with a lagging subscriber", () => + Effect.scoped( + Effect.gen(function*() { + const pubsub = yield* PubSub.sliding({ capacity: 4, replay: 3 }) + yield* PubSub.subscribe(pubsub) + yield* PubSub.publishAll(pubsub, [1, 2]) + const subscription = yield* PubSub.subscribe(pubsub) + yield* PubSub.publishAll(pubsub, [3, 4, 5]) + + const values = yield* PubSub.takeAll(subscription) + assert.isTrue(values.every((value, index) => index === 0 || values[index - 1] <= value)) + }) + )) }) it.effect("shutdown interrupts suspended subscribers", () => @@ -550,3 +606,26 @@ describe("PubSub", () => { }) )) }) + +const retains = (root: object, target: object): boolean => { + const objects = [root] + const seen = new Set() + while (objects.length > 0) { + const current = objects.pop()! + if (current === target) { + return true + } + if (seen.has(current)) { + continue + } + seen.add(current) + for (const key of Reflect.ownKeys(current)) { + const descriptor = Object.getOwnPropertyDescriptor(current, key) + const value = descriptor && "value" in descriptor ? descriptor.value : undefined + if (typeof value === "object" && value !== null) { + objects.push(value) + } + } + } + return false +}