Skip to content

Commit

Permalink
use Map for Scope finalizers, to ensure they are always added (#4432)
Browse files Browse the repository at this point in the history
  • Loading branch information
tim-smart authored Feb 11, 2025
1 parent a2e27e7 commit 3d2e356
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/two-tips-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"effect": patch
---

use Map for Scope finalizers, to ensure they are always added
5 changes: 3 additions & 2 deletions packages/effect/src/internal/effect/circular.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,11 @@ export const forkIn = dual<
core.void :
core.asVoid(core.interruptFiber(fiber))
)
scopeImpl.state.finalizers.add(finalizer)
const key = {}
scopeImpl.state.finalizers.set(key, finalizer)
fiber.addObserver(() => {
if (scopeImpl.state._tag === "Closed") return
scopeImpl.state.finalizers.delete(finalizer)
scopeImpl.state.finalizers.delete(key)
})
} else {
fiber.unsafeInterruptAsFork(parent.id())
Expand Down
13 changes: 7 additions & 6 deletions packages/effect/src/internal/fiberRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3211,7 +3211,7 @@ export const scope: Effect.Effect<Scope.Scope, never, Scope.Scope> = scopeTag
export interface ScopeImpl extends Scope.CloseableScope {
state: {
readonly _tag: "Open"
readonly finalizers: Set<Scope.Scope.Finalizer>
readonly finalizers: Map<{}, Scope.Scope.Finalizer>
} | {
readonly _tag: "Closed"
readonly exit: Exit.Exit<unknown, unknown>
Expand All @@ -3220,7 +3220,7 @@ export interface ScopeImpl extends Scope.CloseableScope {

const scopeUnsafeAddFinalizer = (scope: ScopeImpl, fin: Scope.Scope.Finalizer): void => {
if (scope.state._tag === "Open") {
scope.state.finalizers.add(fin)
scope.state.finalizers.set({}, fin)
}
}

Expand All @@ -3237,12 +3237,13 @@ const ScopeImplProto: Omit<ScopeImpl, "strategy" | "state"> = {
newScope.state = this.state
return newScope
}
const key = {}
const fin = (exit: Exit.Exit<unknown, unknown>) => newScope.close(exit)
this.state.finalizers.add(fin)
this.state.finalizers.set(key, fin)
scopeUnsafeAddFinalizer(newScope, (_) =>
core.sync(() => {
if (this.state._tag === "Open") {
this.state.finalizers.delete(fin)
this.state.finalizers.delete(key)
}
}))
return newScope
Expand Down Expand Up @@ -3297,7 +3298,7 @@ const ScopeImplProto: Omit<ScopeImpl, "strategy" | "state"> = {
if (this.state._tag === "Closed") {
return fin(this.state.exit)
}
this.state.finalizers.add(fin)
this.state.finalizers.set({}, fin)
return core.void
})
}
Expand All @@ -3308,7 +3309,7 @@ const scopeUnsafeMake = (
): ScopeImpl => {
const scope = Object.create(ScopeImplProto)
scope.strategy = strategy
scope.state = { _tag: "Open", finalizers: new Set() }
scope.state = { _tag: "Open", finalizers: new Map() }
return scope
}

Expand Down

0 comments on commit 3d2e356

Please sign in to comment.