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

Use async name resolution on Linux #33378

Open
scalablecory opened this issue Mar 9, 2020 · 8 comments · Fixed by #34633
Open

Use async name resolution on Linux #33378

scalablecory opened this issue Mar 9, 2020 · 8 comments · Fixed by #34633
Assignees
Labels
area-System.Net enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors os-linux Linux OS (any supported distro) tenet-performance Performance related issue
Milestone

Comments

@scalablecory
Copy link
Contributor

scalablecory commented Mar 9, 2020

Edit by @antonfirsov:

this issue has been reopened to track the missing feature, and the related disabled test (see #70089). We should not use getaddrinfo_a as proposed by the original opening comment, since it turned out to be sub-optimal (see #48666).


We should use getaddrinfo_a on Linux for async name resolution. It supports cancellation as well, bringing us in line with Windows support. See sample code for a usage example.

Note: Today we do async over sync on Linux.

@scalablecory scalablecory added enhancement Product code improvement that does NOT require public API changes/additions area-System.Net tenet-performance Performance related issue labels Mar 9, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't add an area label to this Issue.

Checkout this page to find out which area owner to ping, or please add exactly one area label to help train me in the future.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Mar 9, 2020
@karelz karelz removed the untriaged New issue has not been triaged by the area owner label Mar 12, 2020
@karelz karelz added this to the Future milestone Mar 12, 2020
@karelz karelz added the help wanted [up-for-grabs] Good issue for external contributors label Mar 12, 2020
@karelz
Copy link
Member

karelz commented Mar 12, 2020

up-for-grabs: We just need to add function to our PAL layer. Then call it from the C# code.
Should be fairly straightforward -- the Linux code will be very similar to Windows code.

@karelz karelz added the os-linux Linux OS (any supported distro) label Mar 12, 2020
@gfoidl
Copy link
Member

gfoidl commented Apr 1, 2020

I'd like to give this a try.

@gfoidl
Copy link
Member

gfoidl commented Apr 5, 2020

I have a question regarding the workflow for testing the native parts.

Steps I've done (on linux):

  1. building
  2. $ cd src/libraries/System.Net.NameResolution/tests/PalTests
  3. $ pushd ../../src & dotnet build & popd & dotnet build /t:test
  4. verify tests pass
  5. make changes in native code

in a second shell:

  1. $ cd /src/libraries/Native
  2. $ ./build-native.sh debug x64
  3. make sure build is OK

When I test then the changes in native code aren't picked up. Even if i delete libSystem.Native.so then the test pass -- I'd expect them to fail without the native counterpart.

How can I ensure the current libSystem.Native.so is used?

Additional info:
.NET Core 5 SDK was installed before a build of the repo was done.
No custom environment variables got set or any of the default variables got overwritten.

@gfoidl
Copy link
Member

gfoidl commented Apr 5, 2020

LD_DEBUG=libs dotnet build /t:test 2>&1 > /dev/null | grep System.Native

outputs

       678:	calling init: /home/runtime/.dotnet/shared/Microsoft.NETCore.App/5.0.0-preview.3.20169.1/libSystem.Native.so
       701:	calling init: /home/runtime/.dotnet/shared/Microsoft.NETCore.App/5.0.0-preview.3.20169.1/libSystem.Native.so
       701:	calling fini: /home/runtime/.dotnet/shared/Microsoft.NETCore.App/5.0.0-preview.3.20169.1/libSystem.Native.so [0]
       678:	calling fini: /home/runtime/.dotnet/shared/Microsoft.NETCore.App/5.0.0-preview.3.20169.1/libSystem.Native.so [0]

so the native-libs get loaded from the shared installations (from the build) and not from the artifacts-location.

With some hacks* this could be solved, but is there a "nice way" to use the currently built native libraries in the managed tests?

* even LD_PRELOAD doesn't do the job

@stephentoub
Copy link
Member

stephentoub commented Apr 5, 2020

I agree it'd be nice if there were a better solution (and there may be... @ViktorHofer?), but I always just copy the built binaries into the testhost shared folder after building them.

@karelz karelz modified the milestones: Future, 6.0.0 Jan 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 14, 2021
@rzikm
Copy link
Member

rzikm commented Jun 1, 2022

Opening as the async name resolution implementation introduced in #34633 was reverted in #48666

@dotnet dotnet unlocked this conversation Jun 1, 2022
@ViktorHofer
Copy link
Member

so the native-libs get loaded from the shared installations (from the build) and not from the artifacts-location.

That looks like a bug as the testhost should be the single point of truth. @gfoidl can you please open a separate issue for that with repro steps so that we can take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors os-linux Linux OS (any supported distro) tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants