Skip to content

Commit 7df6aee

Browse files
dbrattliclaude
andcommitted
fix(asyncio): await __aexit__ once; split using/tryUsing
Restructure AsyncContextManager so __aexit__ is awaited exactly once on every path. The success-path exit now runs outside the body's try, so a raising __aexit__ propagates instead of triggering a second call. Replace the silent Unchecked.defaultof on suppression with two combinators: - using: always re-raises after __aexit__, returning a plain 'U - tryUsing: honors __aexit__'s suppression signal, returning 'U option Add a hand-written TrackingCm to cover the branches asyncio.Lock can't reach: suppression via tryUsing, using re-raising despite a truthy exit, and no double-exit when the success-path __aexit__ raises. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f696364 commit 7df6aee

2 files changed

Lines changed: 136 additions & 10 deletions

File tree

‎src/stdlib/asyncio/ContextManager.fs‎

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,45 @@ type IAsyncContextManager<'T> =
3232
[<RequireQualifiedAccess>]
3333
module AsyncContextManager =
3434

35+
/// Acquire the resource, run the body, and await `__aexit__` exactly once.
36+
/// Returns `Ok result` when the body succeeds, or `Error (error, suppress)`
37+
/// when it raises — where `suppress` is the truthy/falsy value `__aexit__`
38+
/// returned for that error.
39+
///
40+
/// The body's outcome is captured in an inner `task` so that `__aexit__` is
41+
/// awaited *outside* the `try`. Awaiting the success-path `__aexit__` inside
42+
/// the `try` would route an exception it raises into the error branch and
43+
/// call `__aexit__` a second time — Python's `async with` never does this.
44+
let private run (manager: IAsyncContextManager<'T>) (body: 'T -> Task<'U>) : Task<Result<'U, exn * bool>> =
45+
task {
46+
let! resource = manager.AEnter()
47+
48+
let! outcome =
49+
task {
50+
try
51+
let! result = body resource
52+
return Ok result
53+
with error ->
54+
return Error error
55+
}
56+
57+
match outcome with
58+
| Ok result ->
59+
let! _ = manager.AExit()
60+
return Ok result
61+
| Error error ->
62+
let! suppress = manager.AExit(error)
63+
return Error(error, suppress)
64+
}
65+
3566
/// Run the body within a Python asynchronous context manager, mirroring
3667
/// Python's `async with manager as resource: ...`.
3768
///
3869
/// `__aenter__()` is awaited to acquire the resource, `body` is run with it,
3970
/// and `__aexit__(...)` is awaited afterwards on both the success and error
40-
/// paths. If the body raises and `__aexit__` returns a truthy value the
41-
/// exception is suppressed (as `async with` does); otherwise it is re-raised.
71+
/// paths. If the body raises, the exception is **always re-raised** after
72+
/// `__aexit__` runs — a truthy `__aexit__` return is ignored so the result
73+
/// type stays a plain `'U`. Use `tryUsing` if you need to honor suppression.
4274
///
4375
/// ```fsharp
4476
/// task {
@@ -50,17 +82,28 @@ module AsyncContextManager =
5082
/// ```
5183
let using (manager: IAsyncContextManager<'T>) (body: 'T -> Task<'U>) : Task<'U> =
5284
task {
53-
let! resource = manager.AEnter()
85+
let! outcome = run manager body
5486
55-
try
56-
let! result = body resource
57-
let! _ = manager.AExit()
58-
return result
59-
with error ->
60-
let! suppress = manager.AExit(error)
87+
match outcome with
88+
| Ok result -> return result
89+
| Error(error, _) -> return raise error
90+
}
91+
92+
/// Like `using`, but honors `__aexit__`'s suppression signal — matching the
93+
/// full `async with` contract.
94+
///
95+
/// Returns `Some result` when the body succeeds. If the body raises and
96+
/// `__aexit__` returns a truthy value (the exception is handled), returns
97+
/// `None`; otherwise the exception is re-raised.
98+
let tryUsing (manager: IAsyncContextManager<'T>) (body: 'T -> Task<'U>) : Task<'U option> =
99+
task {
100+
let! outcome = run manager body
61101
102+
match outcome with
103+
| Ok result -> return Some result
104+
| Error(error, suppress) ->
62105
if suppress then
63-
return Unchecked.defaultof<'U>
106+
return None
64107
else
65108
return raise error
66109
}

‎test/TestAsyncIO.fs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,26 @@ type private Lock() =
1111
[<Emit("$0.locked()")>]
1212
member _.locked() : bool = nativeOnly
1313

14+
/// A minimal hand-written async context manager used to exercise the branches
15+
/// that stdlib's asyncio.Lock never hits: __aexit__ returning a truthy value
16+
/// (suppress the exception) and __aexit__ raising on the success path.
17+
/// `exits` counts how many times __aexit__ was awaited.
18+
[<AttachMembers>]
19+
type private TrackingCm(suppress: bool, raiseOnSuccessExit: bool) =
20+
member val exits = 0 with get, set
21+
22+
member this.``__aenter__``() : System.Threading.Tasks.Task<obj> = task { return box this }
23+
24+
member this.``__aexit__``(excType: obj, exc: obj, tb: obj) : System.Threading.Tasks.Task<bool> =
25+
task {
26+
this.exits <- this.exits + 1
27+
28+
if raiseOnSuccessExit && isNull (box exc) then
29+
failwith "exit boom"
30+
31+
return suppress
32+
}
33+
1434
[<Fact>]
1535
let ``test builder run zero works`` () =
1636
let tsk = task { () }
@@ -155,3 +175,66 @@ let ``test async context manager exits on error`` () =
155175
}
156176

157177
asyncio.run tsk |> equal true
178+
179+
[<Fact>]
180+
let ``test tryUsing suppresses error when exit returns true`` () =
181+
let manager = TrackingCm(suppress = true, raiseOnSuccessExit = false)
182+
let cm = unbox<IAsyncContextManager<obj>> manager
183+
184+
let tsk =
185+
task {
186+
// Body raises, but __aexit__ returns true: tryUsing honors the
187+
// suppression and yields None instead of re-raising.
188+
let! result =
189+
AsyncContextManager.tryUsing cm (fun _ ->
190+
task {
191+
do failwith "boom"
192+
return 0
193+
})
194+
195+
return result, manager.exits
196+
}
197+
198+
// No exception escapes, result is None, and __aexit__ ran exactly once.
199+
asyncio.run tsk |> equal (None, 1)
200+
201+
[<Fact>]
202+
let ``test using re-raises even when exit returns true`` () =
203+
let manager = TrackingCm(suppress = true, raiseOnSuccessExit = false)
204+
let cm = unbox<IAsyncContextManager<obj>> manager
205+
206+
let tsk =
207+
task {
208+
try
209+
// `using` always re-raises, ignoring the truthy __aexit__ return.
210+
let! _ =
211+
AsyncContextManager.using cm (fun _ ->
212+
task {
213+
do failwith "boom"
214+
return 0
215+
})
216+
217+
return -1
218+
with _ ->
219+
return manager.exits
220+
}
221+
222+
asyncio.run tsk |> equal 1
223+
224+
[<Fact>]
225+
let ``test async context manager does not exit twice when success exit raises`` () =
226+
let manager = TrackingCm(suppress = false, raiseOnSuccessExit = true)
227+
let cm = unbox<IAsyncContextManager<obj>> manager
228+
229+
let tsk =
230+
task {
231+
try
232+
// Body succeeds; the success-path __aexit__ raises. That error must
233+
// propagate without __aexit__ being invoked a second time.
234+
let! _ = AsyncContextManager.using cm (fun _ -> task { return 0 })
235+
return -1
236+
with _ ->
237+
return manager.exits
238+
}
239+
240+
asyncio.run tsk |> equal 1

0 commit comments

Comments
 (0)