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

Obsolete Socket.UseOnlyOverlappedIO #47163

Closed
antonfirsov opened this issue Jan 19, 2021 · 5 comments · Fixed by #52475
Closed

Obsolete Socket.UseOnlyOverlappedIO #47163

antonfirsov opened this issue Jan 19, 2021 · 5 comments · Fixed by #52475
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Sockets
Milestone

Comments

@antonfirsov
Copy link
Member

antonfirsov commented Jan 19, 2021

Background and Motivation

This property it is only meaningful on .NET Framework where it functions as a socket-level switch between IOCP and Win32 event based overlapped IO.

.NET (Core) Windows sockets are entirely IOCP-based, and the concept of "overlapped IO" does not exist on other platforms, therefore the property contains empty logic, and there is no chance we will ever implement it. To avoid confusion, I suggest to obsolete and hide it it together with the related flag in SocketInformationOptions.

Proposed API

public class Socket
{
+   [Obsolete]
+   [EditorBrowsable(EditorBrowsableState.Never)]
    public bool UseOnlyOverlappedIO { get; set; }
}

public enum SocketInformationOptions
{
    NonBlocking = 1,
    Connected = 2,
    Listening = 4,
+   [Obsolete]
+   [EditorBrowsable(EditorBrowsableState.Never)]
    UseOnlyOverlappedIO = 8,
}

Risks

Breaking /warnaserror builds when porting to .NET 6. (Which may actually prevent runtime failures.)

@antonfirsov antonfirsov added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 19, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net.Sockets untriaged New issue has not been triaged by the area owner labels Jan 19, 2021
@ghost
Copy link

ghost commented Jan 19, 2021

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

Issue Details

Background and Motivation

This property it is only meaningful on .NET Framework where it functions as a ocket-level switch between IOCP and Win32 event based async IO.

.NET (Core) Windows sockets are entirely IOCP-based, and the concept of "overlapped IO" does not exist on other platforms, therefore the property contains empty logic, and there is no chance we will ever implement it. To avoid confusion, I suggest to obsolete it.

Proposed API

public class Socket
{
+   [Obsolete]
    public bool UseOnlyOverlappedIO { get; set; }
}

Risks

Breaking /warnaserror builds when porting to .NET 6. (Which may actually prevent runtime failures.)

Author: antonfirsov
Assignees: -
Labels:

api-suggestion, area-System.Net.Sockets, untriaged

Milestone: -

@Wraith2
Copy link
Contributor

Wraith2 commented Jan 19, 2021

Is it also worth marking it as non browsable?

@antonfirsov
Copy link
Member Author

Is it also worth marking it as non browsable?

In my opinion - yes, so updated the proposal.

@karelz karelz added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Feb 4, 2021
@karelz karelz added this to the 6.0.0 milestone Feb 4, 2021
@karelz
Copy link
Member

karelz commented Feb 4, 2021

Triage: We agree, it is no-op on .NET Core, let's obsolete it.

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Feb 4, 2021
@terrajobst
Copy link
Member

terrajobst commented Feb 4, 2021

Video

  • Looks good
  • It feels over-engineered to add a diagnostic ID to this obsoletion
namespace System.Net.Sockets
{
    public partial class Socket
    {
        [Obsolete]
        [EditorBrowsable(EditorBrowsableState.Never)]
        public bool UseOnlyOverlappedIO { get; set; }
    }
    public partial enum SocketInformationOptions
    {
        [Obsolete]
        [EditorBrowsable(EditorBrowsableState.Never)]
        UseOnlyOverlappedIO = 8,
    }
}

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 7, 2021
antonfirsov pushed a commit that referenced this issue May 18, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 18, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Sockets
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants