consider creating a coreclr version

[grahamehorner]

please consider creating a coreclr implementation/version to allow running on ios and linux

[shahid-pk]

i was looking to port this to coreclr , the first blocking issues that i hit was StringComparison on coreclr does not have InvariantCulture member (will StringComparison.Ordinal work instead ? ) , the second and the bigger problem is StackTrace on coreclr does not have a no argument constructor, meaning you can not get stack frames associated with the current stack (or at least i don't know how to get them without it). Any idea how to deal with these blockers ?

[nayato]

@davidfowl, do you guys have a guidance for such things?

[shahid-pk]

Adding further context to the issue.Without those two blockers DotNetty.Common compiling for coreclr is pretty much trivial. I am compiling DotNetty.Common first because that is what is refrenced in many others places in DotNetty and it also does'nt have dependency on any other things inside DotNetty. I think it will be good if this api is ported to coreclr rather than .net full because if it targets coreclr it will also target .net full and even mono. This kind of decision can be taken now because i think it is not too late as the project has just started. And also targeting only mono and .net full is unnecessarily limiting the reach of this api think first class cross platform support. About the blocker StackTrace issue , the corefx team has agreed to add a no argument constructor but this is not their current priority but still they are open if someone from the community want to add this functionality the related issue is here https://github.com/dotnet/corefx/issues/1797 The code in DotNetty.Common that uses no argument constructor is here https://github.com/Azure/DotNetty/blob/dev/src/DotNetty.Common/ResourceLeakDetector.cs#L346 The other blocker which i think can be worked around or have to work around because the corefx team is not going to add InvariantCulture to StringComparison and they have made their decision related issue is here https://github.com/aspnet/Home/issues/177 And the code that uses invariant culture is here https://github.com/Azure/DotNetty/blob/dev/src/DotNetty.Common/ResourceLeakDetector.cs#L363

[nayato]

@shahid-pk hm.. is this the ctor you mention above? https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Diagnostics/Stacktrace.cs#L182

As for InvariantCulture: https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/StringComparison.cs

Am I looking in wrong places?

[nayato]

btw, on Core CLR support in general: we'd actually want to support .NET Framework 4.5 in the foreseeable future + we would want to move towards Platform Standard https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/standard-platform.md. Per description there, we can start only with version 1.3 for System.Net.Sockets support. 1.3 assumes at least .NET Framework 4.6. I'm not sure how big of an effort it is to have support for both 4.5 and PS. @mgravell has gone through this already, so it would be wise to learn from his experience: http://blog.marcgravell.com/

[shahid-pk]

@nayato coreclr share source with .net full framework so yes those types (StackTrace's no arg constructor and StringComparison's member InvariantCulture) exist in coreclr's mscorlib but these contracts are not exposed in coreclr/corefx surface area.

:edit further more netstandard 1.3 should be good enough for also supporting coreclr as coreclr/corefx implements all the reference assemblies and .net 4.5 doesn't implement any of them but i am not interested in .net 4.5 so for me netstandard 1.3 is good :smile:

[nayato]

StringComparison would come from the same source file I pointed to, it would still have InvariantCulture. Where is the right source file for StringComparison then?..

I imagine that quite a few folks will not be able to upgrade to 4.6 quite yet so whatever you would have to do for Core CLR support would need to work on 4.5 or you'd have to do a bit if #ifs to address Platform Standard (see Marc's blog posts). I.e. for InvariantCulture it makes sense to introduce StringUtil.DefaultCulture and use en-US for Core CLR and CultureInfo.InvariantCulture for .NET Framework. For StackTrace it's even worse: you'd probably need to throw and catch an exception and create stack trace out of that to mimic the same functionality. Sounds like quite a bit of penalty to me.

[shahid-pk]

@nayato

StringComparison would come from the same source file I pointed to, it would still have InvariantCulture. Where is the right source file for StringComparison then?..

The right source file is this one https://github.com/dotnet/corefx/blob/be9efe5d7b21432014a13fffed62d5d99f0466d3/src/System.Runtime/ref/System.Runtime.cs#L1624

As you can see the corefx api does not include InvariantCulture meaning that it does implement the contract but not in one piece :smile:

Thanks about the #if pointer i was exactly going to ask that if this project will except conditional compiling etc.And yes working around the StackTrace will be very problematic and i was also thinking about the same workaround but that seem to me very ugly that is why i asked here,but now it seems that we don't have other options.

[nayato]

ok, this is relevant (whole thread but this comment specifically): https://github.com/dotnet/corefx/issues/1420#issuecomment-94092802 It seems the easiest way is to use Environment.StackTrace and not strip away anything (StackTraceElementExclusions). That would bloat resource leak log but I don't see it as a big concern. Feel free to replace StackFrame array walk with a simple buf.Append(Environment.StackTrace) and no need to do it through #if btw. Or feel free to fix Core FX :)

[shahid-pk]

I am waiting for dotnet/cli tools to get a bit stable once they do i will try to tackle this once again.

[nayato]

@shahid-pk hey, how far did you get with this? What are the showstoppers? Also do you have it up on github (even not compiling is fine)?

[shahid-pk]

@nayato i will upload something to my fork in a few days using dotnet-cli , i was waiting for the dotnet-cli tool and some blocking issues like it being not able to target .net 4.5 which now are solved so i am starting again today. It won't take much time as the source has change very little after that.

[shahid-pk]

here is my local branch https://github.com/shahid-pk/DotNetty/tree/coreclr . It is still not ready for review. I will finish the work as quickly as possible and send a pr.

[nayato]

Great, it doesn't look nearly as bad as UWP :)

[shahid-pk]

Their are two more blocking issues. first ICloneable is not present in .net core and second binary serialization. DotNetty.Transport use ICloneable and things from serialization what is recommended here . ? SerialiazableAttribute,Seriazable and friends are not present in .net core.

[mgravell]

Indeed, BinaryFormatter et al are no more.

Options:

json: newtonsoft or jil XML: XmlSerializer, DataContractSerializer? (neither checked) binary: "bond" (Microsoft), protobuf-net

There are others in each category. On 26 Feb 2016 8:40 p.m., "Shahid Khan" notifications@github.com wrote:

Their are two more blocking issues. first ICloneable is not present in .net core and second binary serialization. DotNetty.Transport use ICloneable and things from serialization what is recommended here . ? SerialiazableAttribute,Seriazable and friends are not present in .net core.

— Reply to this email directly or view it on GitHub https://github.com/Azure/DotNetty/issues/34#issuecomment-189475368.

[nayato]

There is no real value in Serializable and such. Channel ID's NonSerializableAttribute usages, SerializableAttribute on exceptions - just erase them. ICloneable - I only see on AbstractBootstrap - no real value there either - just remove the interface from class declaration and maybe change type of returned object to AbstractBootstrap (as there are no more limitations imposed by interface implementation.

[shahid-pk]

@nayato on to it

[davidfowl]

Good to see progress on this 😀. If you need help let me know

[shahid-pk]

Close method is not available on core for Socket , should i use only Shutdown method ???

[shahid-pk]

used here https://github.com/Azure/DotNetty/blob/dev/src/DotNetty.Transport/Channels/Sockets/TcpSocketChannel.cs#L197 and three more places.

[nayato]

Away from PC at the moment. You'll likely need to call Dispose instead of Close. I'll be able to check semantics a bit later.

On Fri, Feb 26, 2016 at 10:03 PM -0800, "Shahid Khan" notifications@github.com<mailto:notifications@github.com> wrote:

used here https://github.com/Azure/DotNetty/blob/dev/src/DotNetty.Transport/Channels/Sockets/TcpSocketChannel.cs#L197 and three more places.

Reply to this email directly or view it on GitHubhttps://github.com/Azure/DotNetty/issues/34#issuecomment-189582738.

[nayato]

tldr: yes, use Dispose() instead of Close(). Looks like in an absence of Close(int) field _closeTimeout became an obsolete. In fact, looking at socket shutdown recommendations again, it should be OK if not better to not impose timeout of 0 (aka abortive socket closure) and instead rely on predefined linger settings.

[cmello]

Hey I have done a "quick and dirty" port of Buffers, Codecs (with no TlsHandler), Handlers and Transport, on top of shahid's coreclr branch. How can we move this forward, will shahid's branch be pulled into Azure/DotNetty? Thank you very much! It is already working in CoreCLR RC2 for my needs, although slower than .NET 4.6.

[nayato]

@cmello, peeking at your fork, it seems you're not on latest dev. I've done a few changes recently based on portability analyzer report. I haven't gone as far as actually building a thing on .NET Core because it seemed wasteful ahead of final release where project structure will change back to csproj. If you're up for doing this, it's great and I think it won't hurt to add support for RC2 meanwhile. As far as I see, there are outstanding issues with usages of Thread.Join, Stream.Begin/EndRead, ICloneable, Socket.Close, ExecutionContext.SuppressFlow. I can tackle all but the last one in next couple of days (working on TlsHandler changes right now which will resolve Stream.Begin/EndRead). The last one I'd want to keep as is in 4.6 and use #if for .NET Core. Another question here is .NET Platform Standard support and I'd really want to target that with DotNetty. I didn't look much into it but per this, DotNetty should be compatible with v 1.3. To summarize:

• sure we wouldn't mind adding .NET Core RC2 support
• I will accommodate the most of the changes necessary from codebase perspective over the weekend
• it's OK to just add support for .NET Core for now but support for .NET Platform Standard is what we'd be after at the end.

[shahid-pk]

my concerns were same as @nayato that is why i waited for rc-2 release. I will soon make changes keeping in mind @nayato points. @cmello i will update you here about what part i am working so that you could work on other parts or the vise versa.

[cmello]

Thank you guys! Sorry but I don't understand, will .NET Core RTM be Platform Standard 1.5? I'm OK going straight to that end. I was just evaluating performance in a toy benchmark by now, and for my surprise this was my first benchmark that got slower in Core CLR compared to .NET 4.6.

[nayato]

Here's more info: http://dotnet.github.io/docs/core-concepts/libraries/libraries-with-cli.html. Meanwhile I'm having hard time figuring out whether Platform Standard supports X509 and what are the blocking issues right now in the code. Once I'm done with TlsHandler changes (60% rewrite), I'd like to check what fails to build against .NET Core. @shahid-pk, @cmello can you guys rebase on top of current dev meanwhile and see if you can get some compiler output for build against netstandard1.3?

[cmello]

Have you guys seen this? https://blogs.msdn.microsoft.com/dotnet/2016/05/27/making-it-easier-to-port-to-net-core/ .

[nayato]

@shahid-pk, @cmello I've just rebased your changes to latest dev and I'm getting a bunch of errors e.g.:

Your project is not referencing the ".NETFramework,Version=v4.5" framework. Add a reference to ".NETFramework,Version=v4.5" in the "frameworks" section of your project.json, and then re-run NuGet restore.

That's trying to build from VS 2015. Also tried to create a .Net Core console app from scratch and yes, there are things like global.json, xproj project files, etc. Am I missing something or doing smth wrong?

[shahid-pk]

@shahid-pk if you tested my branch i broke the build when i started to move to netstandard1.3 mainly because of some missing reflection api's (i have'nt worked on it since a week will get to it soon) i also removed xproj as i am building on linux also i thought it is too early for the experiment to bother with the overall project. But you can take @cmello fork (although that fork is using old version of dotnet/cli and does not use netstandard) that will build or use command line like i do for my fork, basically go to each folder and do "dotnet build" that way DotNetty.Buffers and DotNetty.Common builds with my changes so far. Next package i am trying to compile with netstandard1.3 is DotNetty.Transport it fails with errors most which i have figured out how to solve. here is the gist for DotNetty.Transport "dotnet build" output. https://gist.github.com/shahid-pk/fd2bfbcac6a6128414829e45992cb8a9

[nayato]

heads up: I've got to setting up VS artifacts. Things are looking good for src projects now. once I'm done with test and example projects, I'll post a PR. Here's progress thus far: https://github.com/nayato/DotNetty/commits/coreclr.

[cmello]

@nayato thank you a lot for working on it. At the time I touched this I had just compiled against the public RC2 to do some bechmarks, without caring to keep compiling for .NET 4.5. But then, if I understood correctly, the changes I made won't be necessary for netstandard1.5, so I will just wait for that. Anyway, if there is anything useful I could do to help, just ask. Thank you a lot! Best regards.

[shahid-pk]

@nayato i was looking at your fork , is their any reason we are targeting both netstandard1.3 and net451 ? i mean if we are cross compiling it anyways why don't we go up to netstandard1.5 ? is it because minimum changes are required between netstandard1.3 and net451?

in any case you are doing a great job :smile:

[nayato]

three reasons:

1. I'm not comfortable enough to switch 100% to .NET Core tooling considering recent announcements etc.
2. Honestly, I haven't tried referencing netstandard1.3 assembly from net451 one. Will try and if that works, we'll likely shut down net451 going forward (but see #3).
3. I #if-ed out suppression of execution context flow which means it will flow again. I've seen something in .NET Core internals regarding this but I didn't follow up on this yet. IIRC, ExecutionContext copying was responsible for some 20+% of GC before suppression was introduced.

[nayato]

OK, watched keynote for dotnetconf. We'll probably reconsider having new tooling only and integrating it in CI. In the meantime, I noticed that netstandard 1.3 is only supported by .NET Framework 4.6+, so would likely need to keep net451 around.

[shahid-pk]

yes i realized that too , netstandard1.3 is no good for net451. thanks for the info

[nayato]

current progress is in https://github.com/nayato/DotNetty/tree/coreclr

[RainingNight]

+1

[nayato]

Addressed with #170

[nayato]

0.4.0 will support .NET Standard 1.3