Skip to content

Bugfix :: Fix F# exception serialization to preserve fields#19342

Merged
T-Gro merged 17 commits into
mainfrom
codegen/fix-exception-serialization
May 14, 2026
Merged

Bugfix :: Fix F# exception serialization to preserve fields#19342
T-Gro merged 17 commits into
mainfrom
codegen/fix-exception-serialization

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented Feb 20, 2026

Fixes F# exceptions losing their field values during serialization.

Exceptions with fields now emit a GetObjectData override and a deserialization constructor that save/restore the fields via SerializationInfo, enabling correct ISerializable roundtrips.

Note: This fix is primarily relevant for users targeting .NET Framework 4.x or .NET Core ≤7, where BinaryFormatter / ISerializable serialization is still available. On .NET 8+, StreamingContext is removed and the serialization members are not emitted. The entire ISerializable infrastructure is deprecated (SYSLIB0051) — no modern serializer (System.Text.Json, DataContractSerializer, etc.) uses GetObjectData.

FSharp.Core exception caveat: FSharp.Core has [assembly: SecurityTransparent], which on .NET Framework prevents overriding the SecurityCritical method Exception.GetObjectData. FSharp.Core exceptions (e.g. MatchFailureException) therefore get only the base-call deserialization constructor (status quo) but not the GetObjectData override. This is tested explicitly. User-defined exceptions are unaffected — they get full serialization support.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 20, 2026

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@T-Gro T-Gro force-pushed the codegen/fix-exception-serialization branch 3 times, most recently from c9b17c0 to f6f7424 Compare February 20, 2026 17:31
@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Feb 20, 2026

/azp run fsharp-ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@T-Gro T-Gro force-pushed the codegen/fix-exception-serialization branch 2 times, most recently from 675d541 to 4b0a72e Compare February 20, 2026 19:29
@T-Gro T-Gro marked this pull request as ready for review March 6, 2026 11:46
@T-Gro T-Gro requested a review from a team as a code owner March 6, 2026 11:46
@T-Gro T-Gro changed the title WIP :: Bugfix :: Fix F# exception serialization to preserve fields Bugfix :: Fix F# exception serialization to preserve fields Mar 6, 2026
@abonie
Copy link
Copy Markdown
Member

abonie commented Mar 23, 2026

An edge case, but now this causes a runtime crash due to field name collision with Exception.GetObjectData keys

open System
open System.Runtime.Serialization

exception Foo of Message:string


let e = Foo("hello")
let info = SerializationInfo(e.GetType(), FormatterConverter())
let ctx = StreamingContext(StreamingContextStates.All)
e.GetObjectData(info, ctx)

@T-Gro T-Gro force-pushed the codegen/fix-exception-serialization branch from 1b2f39a to 2debec9 Compare March 27, 2026 02:42
@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Mar 27, 2026

Good catch! Fixed in 234f944 — serialization keys now use the backing field name (e.g. "Message@" instead of "Message"), which is guaranteed unique due to the @ suffix from CompilerGeneratedName. This avoids the collision with keys written by base.GetObjectData.

Added a dedicated test (Issue_878_ExceptionSerialization_MessageFieldCollision) that exercises exactly your repro scenario and verifies the roundtrip succeeds.

@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Mar 27, 2026

/azp run fsharp-ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@T-Gro T-Gro requested review from abonie and removed request for abonie March 27, 2026 13:58
Comment thread tests/FSharp.Compiler.ComponentTests/CompilerService/AsyncMemoize.fs Outdated
Copy link
Copy Markdown
Member

@abonie abonie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if exception is serialized with an older compiler and deserialized with one that has these changes? If it's not going to work then perhaps this should be marked as a breaking change? Or not because it wasn't working before either?

Comment thread src/Compiler/CodeGen/IlxGen.fs
@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Mar 30, 2026
@T-Gro T-Gro force-pushed the codegen/fix-exception-serialization branch 5 times, most recently from 773eba0 to d485a32 Compare April 20, 2026 14:20
Fixes #878

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro and others added 8 commits April 21, 2026 08:34
Changed eventsWhen condition in 'Cancel running jobs with the same key'
test to wait until all 10 outdated jobs are canceled and the new job
finishes, instead of stopping at the first Finished event. This prevents
race conditions where assertions run before all cancellations complete.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…issue #18411)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use IL backing field name (with @ suffix) as the serialization key
instead of the property name. This prevents duplicate key crashes when
an F# exception field name collides with a key already used by
Exception.GetObjectData (e.g., 'exception Foo of Message:string').

The backing field name (e.g., 'Message@', 'Data0@') is guaranteed
to not collide with base Exception serialization keys.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add Issue_878_ExceptionSerialization_MessageFieldCollision test that
reproduces the exact edge case where an F# exception field named
'Message' collides with Exception.GetObjectData's 'Message' key.

The test verifies:
- GetObjectData does not throw with duplicate key
- F# field is serialized under backing field name 'Message@'
- Base Exception's 'Message' key remains accessible
- Full serialization roundtrip preserves the field value

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Eliminate ilBaseOnlyCtorInstrs/ilBaseOnlyCtor block that duplicated
  ilInstrsForSerialization/ilCtorDefForSerialization by introducing
  shouldRestoreFields flag
- Simplify 3-branch if/elif/else to 2-branch conditional
- Remove unused ilPropName parameter from emitSerializationFieldIL callback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…yIL list

Replace the duplicated getActualIL helper and Assert.Contains call with a
proper IL fragment in the verifyIL list, verifying the AddValue callvirt
instruction is emitted in the GetObjectData method.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rt unrelated AsyncMemoize fix

- Add [CompilerGenerated] and [DebuggerNonUserCode] attributes to the
  generated serialization constructor and GetObjectData override on
  exception types, matching the pattern used by other compiler-generated
  methods in IlxGen.
- Update verifyIL test to check for the new attributes.
- Update all EmittedIL baselines (Nullness, SerializableAttribute) for
  both netcore and net472, debug and release configurations.
- Revert unrelated AsyncMemoize flaky test fix (not part of this PR scope).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the codegen/fix-exception-serialization branch from d485a32 to bb46647 Compare April 21, 2026 06:34
@T-Gro T-Gro added AI-Auto-Resolve-CI Opt-in: LabelOps triages CI failures on this PR every 3h AI-Auto-Resolve-Conflicts Opt-in: LabelOps merges main into this PR and resolves conflicts every 3h labels May 12, 2026
@T-Gro T-Gro enabled auto-merge (squash) May 12, 2026 13:55
Copilot finished work on behalf of T-Gro May 12, 2026 14:07
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot mentioned this pull request May 13, 2026
@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor


Warning

The push_to_pull_request_branch operation failed: Patch size (9759 KB) exceeds maximum allowed size (1024 KB). The code changes were not applied.

🤖 LabelOps — Conflict Resolution.

Merged main and resolved conflicts caused by the baseline duality elimination (#19611):

  • Deleted 8 old-style .debug.bsl / .release.bsl files (superseded by unified .bsl naming)
  • Regenerated 8 unified .bsl baselines with the serialization methods from this PR
  • Auto-merged cleanly: IlxGen.fs, release-notes, ComponentTests.fsproj

✅ Verified: compiler builds, SerializableAttribute tests (4) pass, exception regression tests (36) pass.

Generated by LabelOps — PR Maintenance · ● 25.9M ·

T-Gro and others added 2 commits May 14, 2026 11:57
Resolve 8 modify/delete conflicts: delete old-style .debug.bsl/.release.bsl
files superseded by unified baselines from PR #19611.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After merging main (which eliminated debug/release baseline duality in
PR #19611), regenerate the unified .netcore.bsl baselines to include
the GetObjectData overrides and field-restoring deserialization
constructors from this PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro merged commit 9661324 into main May 14, 2026
50 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in F# Compiler and Tooling May 14, 2026
T-Gro added a commit that referenced this pull request May 15, 2026
…FieldSerializationSupport (F# 11)

The GetObjectData override and field-restoring deserialization constructor
for exception types are now gated behind langversion 11. With langversion
<=10 (the current default), exceptions emit only the base-call ctor
(status quo ante PR #19342).

- Add LanguageFeature.ExceptionFieldSerializationSupport, mapped to F# 11.0
- Gate shouldRestoreFields and GetObjectData emission on the feature
- Update tests to use withLangVersion "11"
- Update all IL baselines (SerializableAttribute, Nullness) to reflect
  the default-langversion output (no field serde IL)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro added a commit that referenced this pull request May 15, 2026
…FieldSerializationSupport (F# 11)

The GetObjectData override and field-restoring deserialization constructor
for exception types are now gated behind langversion 11. With langversion
<=10 (the current default), exceptions emit only the base-call ctor
(status quo ante PR #19342).

- Add LanguageFeature.ExceptionFieldSerializationSupport, mapped to F# 11.0
- Gate shouldRestoreFields and GetObjectData emission on the feature
- Update tests to use withLangVersion "11"
- Update all IL baselines (SerializableAttribute, Nullness) to reflect
  the default-langversion output (no field serde IL)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro added a commit that referenced this pull request May 15, 2026
…FieldSerializationSupport (F# 11)

The GetObjectData override and field-restoring deserialization constructor
for exception types are now gated behind langversion 11. With langversion
<=10 (the current default), exceptions emit only the base-call ctor
(status quo ante PR #19342).

- Add LanguageFeature.ExceptionFieldSerializationSupport, mapped to F# 11.0
- Gate shouldRestoreFields and GetObjectData emission on the feature
- Update tests to use withLangVersion "11"
- Update all IL baselines (SerializableAttribute, Nullness) to reflect
  the default-langversion output (no field serde IL)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Auto-Resolve-CI Opt-in: LabelOps triages CI failures on this PR every 3h AI-Auto-Resolve-Conflicts Opt-in: LabelOps merges main into this PR and resolves conflicts every 3h

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Serialization of F#-defined exception variants doesn't serialize fields

3 participants