Skip to content

Fix the issue that the constructor for UnixDomainSocketEndPoint is unavailable#518

Closed
cshung wants to merge 2 commits intodotnet:masterfrom
cshung:dev/andrewau/fix-unix-domain-socket-unavailable
Closed

Fix the issue that the constructor for UnixDomainSocketEndPoint is unavailable#518
cshung wants to merge 2 commits intodotnet:masterfrom
cshung:dev/andrewau/fix-unix-domain-socket-unavailable

Conversation

@cshung
Copy link
Contributor

@cshung cshung commented Oct 1, 2019

Fixes #307

Fundamentally, we can't use UnixDomainSocketEndPoint if it is not .NET Core 3.0.

Before this change, leveraging anything in this library is impossible for library targetting .NET standard 2.0.

After this change, it is possible to use anything in this library besides the code path I added. In particular, this should unblock the scenario for the user who filed issue #307

If this library is being referenced by an application, @hoyosjs suggested this could be solved by having a package that contains multiple binaries for different target framework monikers. That allows us to have a nice compilation error when unsupported functionalities are used.

However, if this library is going to be referenced by another library (currently targetting .NET standard 2.0) and is in turn referenced by thousands of others. I don't think there is another way round.

We do have customers who want to build that into their application framework that supports all runtime environments. So the scenario above is not fictitious but a real need.

@cshung cshung requested a review from josalem October 1, 2019 21:42
string path = Path.Combine(Path.GetTempPath(), ipcPort);
var remoteEP = new UnixDomainSocketEndPoint(path);
Type unixDomainSocketEndPointType = typeof(Socket).Assembly.GetType("System.Net.Sockets.UnixDomainSocketEndPoint");
var remoteEP = (EndPoint)Activator.CreateInstance(unixDomainSocketEndPointType, new object[]{path});
Copy link
Member

Choose a reason for hiding this comment

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

If the type is null then ArgumentNullException will be thrown. his would happen for Xamarin and Mono. This call is a netcoreapp specific thing. On Mono this call is UnixEndPoint for example. This would require a change to compile for them and play nicely. This change brings value in the sense that a NetFX tool could consume the runtime events or communicate through EventPipe with a 3+ runtime, but I am not sure that this is the right way to achieve that. Multi-TFM packages feel like a more appropriate solution if we don't plan to go away from the socket for unix. As for the transitive dependencies in library scenarios, that's fair and all, but I can't think of a good maintainable way to do this without reflection like here. Maybe someone with more experience in portable managed libraries has a better idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the ArgumentNullException with an extra check.

…gumentNullException in case unixDomainSocketEndPointType == null
@cshung
Copy link
Contributor Author

cshung commented Dec 18, 2019

@hoyosjs I believe this is obsolete now after #700
The only thing we missed in that PR is to check if the type obtained from reflection is null, I think we should do that.

@sywhang
Copy link
Contributor

sywhang commented Dec 18, 2019

@cshung yes. But since this PR is targeting the old code (RuntimeClientLibrary), I think I'll just close this PR and submit a new one if that's ok with you @cshung?

@sywhang sywhang closed this Dec 18, 2019
@cshung cshung deleted the dev/andrewau/fix-unix-domain-socket-unavailable branch December 18, 2019 18:33
@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to use IpcClient under .NET Framework 4.7.1

3 participants