Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use File.OpenHandle in Socket.SendFile directly #56777

Merged
merged 4 commits into from
Aug 3, 2021

Conversation

huoyaoyuan
Copy link
Member

Saves a FileStream allocation.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 3, 2021
@ghost
Copy link

ghost commented Aug 3, 2021

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

Issue Details

Saves a FileStream allocation.

Author: huoyaoyuan
Assignees: -
Labels:

area-System.Net.Sockets, community-contribution

Milestone: -

@@ -204,11 +204,14 @@ private void SendFileInternal(string? fileName, ReadOnlySpan<byte> preBuffer, Re
Send(preBuffer);
}

// Send the file, if any
if (fileHandle != null)
using (fileHandle)
Copy link
Member

@stephentoub stephentoub Aug 3, 2021

Choose a reason for hiding this comment

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

This should be done earlier. The preBuffer sending above should be a part of this so that the fileHandle will be disposed if that Send throws.

Comment on lines 401 to 408
SafeFileHandle? fileHandle = OpenFileHandle(fileName);

SocketError errorCode;
// This can throw ObjectDisposedException.
errorCode = SocketPal.SendFile(_handle, fileHandle, preBuffer, postBuffer, flags);

using (fileHandle)
{
// This can throw ObjectDisposedException.
errorCode = SocketPal.SendFile(_handle, fileHandle, preBuffer, postBuffer, flags);
}
Copy link
Member

Choose a reason for hiding this comment

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

This can be:

SocketError errorCode;
using (SafeFileHandle? fileHandle = OpenFileHandle(fileName))
{
    // This can throw ObjectDisposedException.
    errorCode = SocketPal.SendFile(_handle, fileHandle, preBuffer, postBuffer, flags);
}

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks!

@stephentoub stephentoub merged commit 4b3db59 into dotnet:main Aug 3, 2021
thaystg added a commit to thaystg/runtime that referenced this pull request Aug 3, 2021
# By Camillo Toselli (1) and others
# Via GitHub
* origin/main:
  add RID for Debian 11 (dotnet#56789)
  [wasm] [debugger] Skip thread static field (dotnet#56749)
  Fix timeouts in coreroot_determinism test in GC stress mode (dotnet#56770)
  Use File.OpenHandle in Socket.SendFile directly (dotnet#56777)
  accept empty realm for digest auth (dotnet#56369) (dotnet#56455)

# Conflicts:
#	src/mono/wasm/debugger/DebuggerTestSuite/BreakpointTests.cs
#	src/mono/wasm/debugger/DebuggerTestSuite/GetPropertiesTests.cs
thaystg added a commit to thaystg/runtime that referenced this pull request Aug 4, 2021
…ger_proxy_attribute

* origin/main: (340 commits)
  add RID for Debian 11 (dotnet#56789)
  [wasm] [debugger] Skip thread static field (dotnet#56749)
  Fix timeouts in coreroot_determinism test in GC stress mode (dotnet#56770)
  Use File.OpenHandle in Socket.SendFile directly (dotnet#56777)
  accept empty realm for digest auth (dotnet#56369) (dotnet#56455)
  [wasm][debugger] Create test Inherited Properties (dotnet#56754)
  Mark new test as incompatible with GC Mark4781_1GcStressIncompatible (dotnet#56739)
  Ensure MetadataEnumResult is sufficiently updated by MetaDataImport::Enum (dotnet#56756)
  [mono] Remove gdb xdebug and binary writer support, it hasn't worked in a while. (dotnet#56759)
  Update windows-requirements.md (dotnet#56476)
  Update doc and generic parameter name for JsonValue.GetValue (dotnet#56639)
  [wasm][debugger] Inspect static class (dotnet#56740)
  Fix stack overflow handling issue in GC stress (dotnet#56733)
  Use ReflectionOnly as serialization mode in case dynamic code runtime feature is not supported (dotnet#56604)
  Move Windows Compat pack to NuGet pack task (dotnet#56686)
  Fix build error when building some packages (dotnet#56767)
  Simplify JIT shutdown logic in crossgen2 (dotnet#56687)
  Fix race in crossdac publishing with PGO (dotnet#56762)
  Add DictionaryKeyPolicy support for EnumConverter [dotnet#47765] (dotnet#54429)
  Use ComWrappers in some Marshal unit-tests and update platform metadata  (dotnet#56595)
  ...
@huoyaoyuan huoyaoyuan deleted the socket-filehandle branch August 4, 2021 03:35
@karelz karelz added this to the 6.0.0 milestone Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Sockets community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants