Skip to content

Nullability annotations for HttpListener - #41189

Merged
pgovind merged 3 commits into
dotnet:masterfrom
pgovind:nullability_httplistener
Aug 26, 2020
Merged

Nullability annotations for HttpListener#41189
pgovind merged 3 commits into
dotnet:masterfrom
pgovind:nullability_httplistener

Conversation

@pgovind

@pgovindpgovind commented Aug 21, 2020

Copy link
Copy Markdown

@Dotnet-GitSync-BotDotnet-GitSync-Bot added the area-Infrastructure-coreclr Only use for closed issues label Aug 21, 2020
@pgovindpgovind added area-System.Net.Http and removed area-Infrastructure-coreclr Only use for closed issues labels Aug 21, 2020
@ghost

ghost commented Aug 21, 2020

Copy link
Copy Markdown

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

lindexi
lindexi reviewed Aug 22, 2020
View reviewed changes
Comment threadsrc/libraries/System.Net.HttpListener/src/System/Net/HttpListenerRequest.cs Outdated
lindexi
lindexi reviewed Aug 22, 2020
View reviewed changes
Comment threadsrc/libraries/System.Net.HttpListener/src/System/Net/Managed/ListenerAsyncResult.Managed.cs Outdated
aik-jahoda
aik-jahoda reviewed Aug 25, 2020
View reviewed changes

@aik-jahodaaik-jahoda 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.

Added few comments.

Comment threadsrc/libraries/System.Net.HttpListener/ref/System.Net.HttpListener.cs
Comment threadsrc/libraries/System.Net.HttpListener/src/System/Net/HttpListenerRequest.cs Outdated
Comment threadsrc/libraries/System.Net.HttpListener/src/System/Net/Managed/HttpConnection.cs Outdated
Comment threadsrc/libraries/System.Net.HttpListener/src/System/Net/Managed/HttpConnection.cs
Comment threadsrc/libraries/System.Net.HttpListener/src/System/Net/Managed/ListenerAsyncResult.Managed.cs Outdated
@carlossanlopcarlossanlop mentioned this pull request Aug 25, 2020
Merged
14 tasks
carlossanlop
carlossanlop reviewed Aug 25, 2020
View reviewed changes
Comment threadsrc/libraries/System.Net.HttpListener/src/System/Net/Managed/HttpConnection.cs
int length = (int)_memoryStream.Length;
byte[] buffer = _memoryStream!.GetBuffer();
int length = (int)_memoryStream!.Length;
_memoryStream = null;

@carlossanlopcarlossanlopAug 25, 2020•
edited
Loading

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.

@aik-jahoda shouldn't we call _memoryStream.Dispose() here? I find it weird that it is being set to null.

I ask because this is what forced Prashanth to make _memoryStream nullable.

@aik-jahodaaik-jahodaAug 26, 2020

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.

I wouldn't change behaviour now but create follow up work for .NET 6. I'm fine with nullable _memoryStream since we have this discussion recorder here for future fix.

aik-jahoda
aik-jahoda approved these changes Aug 26, 2020
View reviewed changes

@aik-jahodaaik-jahoda 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.

LGTM

@pgovind
pgovind merged commit 86224fd into dotnet:masterAug 26, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@karelzkarelz added this to the 6.0.0 milestone Jan 26, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers
@carlossanlopcarlossanlopcarlossanlop left review comments
@lindexilindexilindexi left review comments
@aik-jahodaaik-jahodaaik-jahoda approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Net.Http

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@pgovind@carlossanlop@aik-jahoda@lindexi@karelz@Dotnet-GitSync-Bot