Skip to content

[native] Skip F_FULLFSYNC on macOS for network file systems#130779

Open
adamsitnik with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-network-file-system-locking
Open

[native] Skip F_FULLFSYNC on macOS for network file systems#130779
adamsitnik with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-network-file-system-locking

Conversation

Copilot AI commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

On macOS, F_FULLFSYNC on network file systems (NFS, SMB, CIFS, SMB2) can return success while silently discarding pending writes, causing data corruption. The existing fsync fallback only triggers on failure, so the data loss goes undetected.

Changes in src/native/libs/System.Native/pal_io.c:

  • Added IsNetworkFileSystem(int fileDescriptor) — macOS-only static helper that calls fstatfs and reuses the existing FileSystemNameSupportsLocking logic to identify NFS/CIFS/SMB/SMB2 mounts. Conservatively returns 1 (is network FS) when fstatfs fails, to skip F_FULLFSYNC and avoid data loss.
  • Modified SystemNative_FSync to skip F_FULLFSYNC entirely when IsNetworkFileSystem returns true, going directly to fsync. Behavior on local file systems is unchanged.

This mirrors the approach already used by CanLockTheFile / SystemNative_FileSystemSupportsLocking, which skip advisory locking on the same set of network file systems for the same category of reasons.

Copilot AI and others added 2 commits July 15, 2026 13:14
Don't use F_FULLFSYNC on macOS for network file systems (NFS, SMB, CIFS, SMB2)
as it may cause pending writes to be discarded. Use the same detection logic
already used by the locking code (fstatfs + FileSystemNameSupportsLocking).

Fixes #124722

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
On fstatfs failure, return 1 (treat as network FS) to safely skip F_FULLFSYNC
and avoid potential data loss. Also improve comment clarity on return value logic.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI self-assigned this Jul 15, 2026
Copilot AI review requested due to automatic review settings July 15, 2026 13:16
Copilot AI removed the request for review from Copilot July 15, 2026 13:16
Copilot AI requested a review from adamsitnik July 15, 2026 13:17
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot please address my feedback

Comment thread src/native/libs/System.Native/pal_io.c Outdated
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 13:46
…pal_io.c

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from adamsitnik and Copilot and removed request for Copilot July 15, 2026 13:48

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot address my feedback

Comment thread src/native/libs/System.Native/pal_io.c Outdated
Comment thread src/native/libs/System.Native/pal_io.h Outdated
Comment thread src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems Outdated
Comment thread src/native/libs/System.Native/entrypoints.c Outdated
…ckTheFile in managed code

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 15:10
…ix WASM FSync signature

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from adamsitnik and Copilot and removed request for Copilot July 15, 2026 16:32
Copilot AI temporarily deployed to copilot-pat-pool July 15, 2026 16:36 Inactive
@adamsitnik
adamsitnik marked this pull request as ready for review July 15, 2026 16:36
Copilot AI review requested due to automatic review settings July 15, 2026 16:36
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread src/native/libs/System.Native/pal_io.c

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot address feedback

Comment thread src/native/libs/System.Native/pal_io.c
…avior

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@adamsitnik
adamsitnik requested a review from jozkee July 16, 2026 12:48
@adamsitnik

Copy link
Copy Markdown
Member

@jozkee the PR is ready for review, PTAL. #124722 is the context

get
{
NullableBool useFullFsync = _useFullFsync;
if (useFullFsync == NullableBool.Undefined && !IsClosed && OperatingSystem.IsApplePlatform())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SystemNative_FSync uses TARGET_OSX only.

Suggested change
if (useFullFsync == NullableBool.Undefined && !IsClosed && OperatingSystem.IsApplePlatform())
if (useFullFsync == NullableBool.Undefined && !IsClosed && OperatingSystem.IsMacOS())

NullableBool useFullFsync = _useFullFsync;
if (useFullFsync == NullableBool.Undefined && !IsClosed && OperatingSystem.IsApplePlatform())
{
_useFullFsync = useFullFsync = Interop.Sys.FileSystemSupportsLocking(this, Interop.Sys.LockOperations.LOCK_SH, accessWrite: true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The PR reuses SystemNative_FileSystemSupportsLocking as a proxy for "is this a network FS," but that function's contract is about locking and its result depends on lockOperation/accessWrite, so it can't simply be renamed to IsNetworkFileSystem. Consider extracting the shared network-FS detection (the FileSystemNameSupportsLocking nfs/cifs/smb/smb2 check) into a dedicated helper.

@jozkee

jozkee commented Jul 18, 2026

Copy link
Copy Markdown
Member

I tried to reproduce #124722 on a macOS client (macOS 26.5, arm64) connecting to a Samba server running in WSL2, but was not able to trigger the truncation.

What I tried:

  • The exact repro from the issue (16 × 64 MB,  FileShare.None  +  Flush(flushToDisk: true) ), plus larger sizes (256 MB × 16 and 1 GB × 4) and multiple runs.
  • Sequential, async, and fully-parallel (concurrent) writes.
  • Verifying final file sizes three ways to rule out the macOS SMB client's metadata cache: post-close  stat , a full byte-for-byte read-back from a separate process, and a force-unmount/remount followed by a fresh  stat .

In every case all files came out at their exact expected size — 0 truncated.

@adamsitnik

Copy link
Copy Markdown
Member

I tried to reproduce #124722 on a macOS client (macOS 26.5, arm64) connecting to a Samba server running in WSL2, but was not able to trigger the truncation.

What I tried:

  • The exact repro from the issue (16 × 64 MB,  FileShare.None  +  Flush(flushToDisk: true) ), plus larger sizes (256 MB × 16 and 1 GB × 4) and multiple runs.
  • Sequential, async, and fully-parallel (concurrent) writes.
  • Verifying final file sizes three ways to rule out the macOS SMB client's metadata cache: post-close  stat , a full byte-for-byte read-back from a separate process, and a force-unmount/remount followed by a fresh  stat .

In every case all files came out at their exact expected size — 0 truncated.

@sschultze is there something more specific required to reproduce the problem (#124722)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FileStream.Flush(flushToDisk: true) causes corrupt (truncated) files to be written on macOS + SMB share

4 participants