Skip to content

Commit cb87e57

Browse files
T-GroCopilot
andauthored
Fix seq try-with handler body executing twice (#19660) (#19661)
Guard disposeOriginal() against accessing a faulted Lazy<T> in EnumerateTryWith. When source.GetEnumerator() throws, the Lazy caches the exception and re-throws on every .Value access, causing the disposal path to re-trigger moveExceptionHandler. Add IsValueCreated check so disposal is skipped when no enumerator was successfully created. Fixes #19660 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 836319f commit cb87e57

3 files changed

Lines changed: 58 additions & 1 deletion

File tree

docs/release-notes/.FSharp.Core/10.0.300.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* Fix EvaluateQuotation to handle Sequential expressions, void method calls (unit return), and other patterns that were previously throwing NotSupportedException. Also properly handles unit-returning expressions by using Action delegates instead of Func delegates. ([Issue #19099](https://github.com/dotnet/fsharp/issues/19099))
99
* Fix query conditionals without else branch (if-then only) that were causing type mismatch errors. Now properly extracts element type from IQueryable for creating empty sequences. ([Issue #3445](https://github.com/dotnet/fsharp/issues/3445))
1010
* Fix `Seq.empty` rendering as `"EmptyEnumerable"` in serializers by delegating to `System.Linq.Enumerable.Empty<'T>()` instead of using a custom DU type. ([Issue #17864](https://github.com/dotnet/fsharp/issues/17864), [PR #19317](https://github.com/dotnet/fsharp/pull/19317))
11+
* Fix `seq { try/with }` handler body executing twice when source throws immediately and handler yields nothing. ([Issue #19660](https://github.com/dotnet/fsharp/issues/19660), [PR #19661](https://github.com/dotnet/fsharp/pull/19661))
1112
* Ensure culture-independent parsing of .NET-style interpolated string holes. ([Issue #19367](https://github.com/dotnet/fsharp/issues/19367), [PR #19370](https://github.com/dotnet/fsharp/pull/19370))
1213

1314
### Added

src/FSharp.Core/seqcore.fs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,8 @@ module RuntimeHelpers =
395395
let disposeOriginal() =
396396
if shouldDisposeOriginalAtTheEnd then
397397
shouldDisposeOriginalAtTheEnd <- false
398-
originalSource.Value.Dispose()
398+
if originalSource.IsValueCreated then
399+
originalSource.Value.Dispose()
399400

400401
let moveExceptionHandler exn =
401402
exceptionalSource <- Some ((exceptionHandler exn).GetEnumerator())

tests/FSharp.Compiler.ComponentTests/Language/SequenceExpressions/SequenceExpressionTests.fs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,61 @@ let whatIsIt =
386386
|> runCode
387387
|> shouldSucceed
388388

389+
// Regression test for https://github.com/dotnet/fsharp/issues/19660
390+
[<Fact>]
391+
let ``Handler body executes once when source throws immediately and handler yields nothing``() =
392+
Fsx """
393+
let mutable bodyCount = 0
394+
let result =
395+
seq {
396+
try yield (1/0)
397+
with _ ->
398+
bodyCount <- bodyCount + 1
399+
()
400+
} |> Array.ofSeq
401+
if bodyCount <> 1 then failwith $"bodyCount was {bodyCount}"
402+
if result <> [||] then failwith $"result was %A{result}"
403+
"""
404+
|> runCode
405+
|> shouldSucceed
406+
407+
// When-guard double-execution is by design (RFC FS-1134 L37-38).
408+
409+
[<Fact>]
410+
let ``When guard in seq try-with - false guard falls through correctly``() =
411+
Fsx """
412+
let mutable guard1Count = 0
413+
let mutable guard2Count = 0
414+
let result =
415+
seq {
416+
try yield (1/0)
417+
with
418+
| _ when (guard1Count <- guard1Count + 1; false) -> yield 1
419+
| _ when (guard2Count <- guard2Count + 1; true) -> yield 99
420+
} |> Array.ofSeq
421+
if guard1Count <> 2 then failwith $"guard1Count was {guard1Count}"
422+
if guard2Count <> 2 then failwith $"guard2Count was {guard2Count}"
423+
if result <> [|99|] then failwith $"result was %A{result}"
424+
"""
425+
|> runCode
426+
|> shouldSucceed
427+
428+
[<Fact>]
429+
let ``When guard in seq try-with executes twice per iteration in for loop``() =
430+
Fsx """
431+
let mutable guardCount = 0
432+
let result =
433+
seq {
434+
for x in [0; 0; 0] do
435+
try yield (1/x)
436+
with _ when (guardCount <- guardCount + 1; true) -> yield 99
437+
} |> Array.ofSeq
438+
if guardCount <> 6 then failwith $"guardCount was {guardCount}"
439+
if result <> [|99; 99; 99|] then failwith $"result was %A{result}"
440+
"""
441+
|> runCode
442+
|> shouldSucceed
443+
389444
[<Theory>]
390445
[<InlineData("41","42","43")>]
391446
[<InlineData("()","42","43")>]

0 commit comments

Comments
 (0)