Add TryGetOwnedMemory ref and tests #27288

benaadams · 2018-02-20T19:56:48Z

Resolves https://github.com/dotnet/corefx/issues/27237

stephentoub · 2018-02-20T20:55:24Z

src/Common/src/CoreLib/System/Runtime/InteropServices/MemoryMarshal.cs

+            }
+
+            ownedMemory = default;
+            return false;


How about:

TOwner owner = readOnlyMemory._object as TOwner; ownedMemory = owner; return owner != null;

?

stephentoub · 2018-02-20T20:57:09Z

src/System.Memory/ref/System.Memory.cs

 }
 namespace System.Runtime.InteropServices
 {
+    using System.Buffers;


We don't do this in contracts. When this file is auto-generated by the tool at some point again, this will be removed and full names will be used everywhere. Might as well do so now.

Was having issues getting it compile; didn't know needed a change to coreclr also/first dotnet/coreclr#16455

stephentoub · 2018-02-20T20:57:39Z

src/Common/src/CoreLib/System/Runtime/InteropServices/MemoryMarshal.cs

+                return true;
+            }
+
+            ownedMemory = default;


default => null?

KrzysztofCwalina · 2018-02-20T21:51:42Z

src/System.Memory/tests/MemoryMarshal/TryGetOwnedMemory.cs

+            }
+        }
+
+        internal class OtherMemoryForTest<T> : OwnedMemory<T>


Why does the name start with "Other"?

There is a MemoryForTest this one is just for testing that it doesn't cast to it. Have a preferred name?

stephentoub · 2018-02-20T21:53:44Z

src/Common/src/CoreLib/System/Runtime/InteropServices/MemoryMarshal.cs

+        public static bool TryGetOwnedMemory<T, TOwner>(ReadOnlyMemory<T> readOnlyMemory, out TOwner ownedMemory)
+            where TOwner : OwnedMemory<T>
+        {
+            TOwner owner; // Use register for null comparision rahter than byref


Nit: rahter => rather

Nit: comparision => comparison

benaadams · 2018-02-21T01:59:50Z

Squashed, as I assume it will have to go with a coreclr merge

stephentoub · 2018-02-21T02:01:18Z

Squashed, as I assume it will have to go with a coreclr merge

It shouldn't. Nothing in the coreclr change introduced a break with corefx, did it? Assuming not, once that is consumed, this can just be re-run and then merged.

benaadams · 2018-02-21T02:04:46Z

Doesn't it complain there is an implementation with no declaration? Or has that gone now?

benaadams · 2018-02-21T02:05:26Z

Ah... Have to do something with this #27309 (comment)

stephentoub · 2018-02-21T02:06:18Z

Doesn't it complain there is an implementation with no declaration? Or has that gone now?

I don't think it's ever complained about that. It definitely complains if there's something specified in the contract that doesn't exist in the implementation, but not the other way around.

benaadams · 2018-02-21T04:29:14Z

K, think worked out how this mirroring works :)

benaadams · 2018-02-21T08:47:22Z

Rebooting

benaadams · 2018-02-21T08:48:04Z

@dotnet-bot test OSX x64 Debug Build
@dotnet-bot test Windows x86 Release Build

benaadams · 2018-02-21T23:42:23Z

OwnedMemoryBit needs to be removed from the index dotnet/coreclr#16479

benaadams · 2018-02-22T21:02:57Z

NETFX unrelated

System.Net.Http.Functional.Tests.HttpCookieProtocolTests.GetAsyncWithRedirect_SetCookieContainer_CorrectCookiesSent [FAIL]
  System.Net.Http.HttpRequestException : An error occurred while sending the request.
  ---- System.Net.WebException : The request was aborted: The request was canceled.
  Stack Trace:
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Net.Http.HttpClient.<FinishSendAsyncBuffered>d__58.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Net.Http.Functional.Tests.HttpCookieProtocolTests.<<GetAsyncWithRedirect_SetCookieContainer_CorrectCookiesSent>b__15_0>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Net.Test.Common.LoopbackServer.<>c__DisplayClass11_0.<<CreateClientAndServerAsync>b__0>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Net.Test.Common.LoopbackServer.<CreateServerAsync>d__9.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Net.Http.Functional.Tests.HttpCookieProtocolTests.<GetAsyncWithRedirect_SetCookieContainer_CorrectCookiesSent>d__15.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    ----- Inner Stack Trace -----
       at System.Net.HttpWebRequest.EndGetResponse(IAsyncResult asyncResult)
       at System.Net.Http.HttpClientHandler.GetResponseCallback(IAsyncResult ar)

benaadams · 2018-02-22T23:36:38Z

@dotnet-bot test NETFX x86 Release Build

benaadams · 2018-02-23T00:31:53Z

@stephentoub should I raise an issue for the NETFX issue or is it just transient networking?

stephentoub · 2018-02-23T02:11:22Z

should I raise an issue for the NETFX issue or is it just transient networking?

Please open an issue. I don't believe we've seen that one fail before, and it's using a loopback server, so really shouldn't have any significant transient networking issues. Thanks.

Commit migrated from dotnet/corefx@064a638

benaadams force-pushed the TryGetOwnedMemory branch 3 times, most recently from faa6219 to c6f08e5 Compare February 20, 2018 20:52

stephentoub reviewed Feb 20, 2018

View reviewed changes

benaadams mentioned this pull request Feb 20, 2018

Add TryGetOwnedMemory dotnet/coreclr#16455

Merged

joshfree assigned ahsonkhan and KrzysztofCwalina Feb 20, 2018

KrzysztofCwalina reviewed Feb 20, 2018

View reviewed changes

KrzysztofCwalina approved these changes Feb 20, 2018

View reviewed changes

stephentoub reviewed Feb 20, 2018

View reviewed changes

benaadams force-pushed the TryGetOwnedMemory branch from 7341065 to 6c849e3 Compare February 21, 2018 01:59

benaadams changed the title ~~Add TryGetOwnedMemory~~ Add TryGetOwnedMemory ref and tests Feb 21, 2018

benaadams closed this Feb 21, 2018

benaadams reopened this Feb 21, 2018

benaadams force-pushed the TryGetOwnedMemory branch from a8385b4 to dfe131c Compare February 21, 2018 17:14

benaadams closed this Feb 21, 2018

benaadams reopened this Feb 21, 2018

benaadams force-pushed the TryGetOwnedMemory branch from dfe131c to 6cbd1c8 Compare February 22, 2018 18:58

Add TryGetOwnedMemory tests and ref

f214f42

benaadams force-pushed the TryGetOwnedMemory branch from 6cbd1c8 to f214f42 Compare February 22, 2018 20:30

ahsonkhan mentioned this pull request Feb 23, 2018

TryGet for ReadOnlysequence #27229

Merged

stephentoub merged commit 064a638 into dotnet:master Feb 23, 2018

benaadams deleted the TryGetOwnedMemory branch February 23, 2018 02:17

karelz added this to the 2.1.0 milestone Mar 10, 2018

benaadams mentioned this pull request Jan 31, 2020

System.Net.Http.Functional.Tests.HttpCookieProtocolTests.GetAsyncWithRedirect_SetCookieContainer_CorrectCookiesSent [FAIL] dotnet/runtime#25158

Closed

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Add TryGetOwnedMemory tests and ref (dotnet/corefx#27288)

eebef1d

Commit migrated from dotnet/corefx@064a638

Add TryGetOwnedMemory ref and tests #27288

Add TryGetOwnedMemory ref and tests #27288

Uh oh!

Conversation

benaadams commented Feb 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benaadams commented Feb 21, 2018

Uh oh!

stephentoub commented Feb 21, 2018

Uh oh!

benaadams commented Feb 21, 2018

Uh oh!

benaadams commented Feb 21, 2018

Uh oh!

stephentoub commented Feb 21, 2018

Uh oh!

benaadams commented Feb 21, 2018

Uh oh!

benaadams commented Feb 21, 2018

Uh oh!

benaadams commented Feb 21, 2018

Uh oh!

benaadams commented Feb 21, 2018

Uh oh!

benaadams commented Feb 22, 2018

Uh oh!

benaadams commented Feb 22, 2018

Uh oh!

benaadams commented Feb 23, 2018

Uh oh!

stephentoub commented Feb 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants