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

NameResolutionPal.Unix enabled async name resolution #34633

Merged
merged 37 commits into from
Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
fc09ca1
Native implementation
gfoidl Apr 6, 2020
8c955bc
Native tests
gfoidl Apr 6, 2020
cf13c70
Managed implementation
gfoidl Apr 6, 2020
4614267
Managed tests
gfoidl Apr 6, 2020
1b2daab
Revert Interop.GetHostName change -- it's needed at other places too
gfoidl Apr 7, 2020
b9629e9
Fixed builds failures due to "unused parameter"
gfoidl Apr 7, 2020
6f64c86
Native: move struct addrinfo hint from stack-alloc to heap-alloc
gfoidl Apr 7, 2020
e7df0cc
PR feedback
gfoidl Apr 8, 2020
d7cb2be
Fixed leak in tests.c according to valgrind's run
gfoidl Apr 9, 2020
74b3031
Fixed bug due to marshalled string argument
gfoidl Apr 9, 2020
775ff72
More managed PalTests
gfoidl Apr 9, 2020
ec8522f
Revert "Native tests"
gfoidl Apr 9, 2020
298cf03
Handle the case of HostName="" and tests for this
gfoidl Apr 9, 2020
458579b
Use flexible array member to hold the address in struct state
gfoidl Apr 9, 2020
8f69164
Nits in native layer
gfoidl Apr 10, 2020
8cec5fb
Merge branch 'master' into unix-async-name-resolution
gfoidl Jul 1, 2020
f3ebd6a
Merge branch 'master' into unix-async-name-resolution
gfoidl Aug 13, 2020
cf45cc9
Merge branch 'master' into unix-async-name-resolution
gfoidl Dec 15, 2020
43f889f
Fixed native merge conflict + added AddressFamily-handling
gfoidl Dec 15, 2020
bd16c74
Fixed managed merge conflicts + added AddressFamily-handling
gfoidl Dec 15, 2020
ed5c39f
Updated NameResolutionPalTests for AddressFamily
gfoidl Dec 15, 2020
b8efde5
Removed EnsureSocketsAreInitialized
gfoidl Dec 15, 2020
04dbd8e
Fixed bug at native side
gfoidl Dec 15, 2020
37e4dd3
Refactor setting of the address family into TrySetAddressFamily
gfoidl Dec 15, 2020
7176378
Fixed unused parameter warning / failure
gfoidl Dec 15, 2020
b64867b
Little cleanup
gfoidl Dec 16, 2020
900834e
Fixed (unrelated) test failures
gfoidl Dec 16, 2020
198717e
Made LoggingTest async instead of GetAwaiter().GetResult()
gfoidl Dec 16, 2020
f85c316
Use function pointer
gfoidl Dec 16, 2020
438c75a
Use OperatinngSystem instead of RuntimeInformation in tests
gfoidl Dec 18, 2020
89c266f
Merge branch 'master' into unix-async-name-resolution
gfoidl Dec 18, 2020
33da8da
PR Feedback for CMake
gfoidl Dec 18, 2020
fa9c6f9
Update src/libraries/Native/Unix/System.Native/extra_libs.cmake
VSadov Dec 18, 2020
8266dd3
Revert "Update src/libraries/Native/Unix/System.Native/extra_libs.cmake"
gfoidl Jan 8, 2021
75e52c7
Another to build single file host
gfoidl Jan 8, 2021
89a7c5a
Merge branch 'master' into unix-async-name-resolution
gfoidl Jan 14, 2021
dd1e245
Test for !Windows instead excluding several Unix-flavors
gfoidl Jan 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/installer/corehost/cli/apphost/static/configure.cmake
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
include(CheckIncludeFiles)
include(CMakePushCheckState)

check_include_files(
GSS/GSS.h
Expand All @@ -10,3 +11,16 @@ if (HeimdalGssApi)
gssapi/gssapi.h
HAVE_HEIMDAL_HEADERS)
endif()

if (CLR_CMAKE_TARGET_LINUX)
cmake_push_check_state(RESET)
set (CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE")
set (CMAKE_REQUIRED_LIBRARIES "-lanl")

check_symbol_exists(
getaddrinfo_a
netdb.h
HAVE_GETADDRINFO_A)

cmake_pop_check_state()
endif ()
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Net.Sockets;
using System.Runtime.InteropServices;

internal static partial class Interop
Expand Down Expand Up @@ -32,8 +32,18 @@ internal unsafe struct HostEntry
internal int IPAddressCount; // Number of IP addresses in the list
}

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_PlatformSupportsGetAddrInfoAsync")]
internal static extern bool PlatformSupportsGetAddrInfoAsync();

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetHostEntryForName")]
internal static extern unsafe int GetHostEntryForName(string address, System.Net.Sockets.AddressFamily family, HostEntry* entry);
internal static extern unsafe int GetHostEntryForName(string address, AddressFamily family, HostEntry* entry);

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetHostEntryForNameAsync")]
internal static extern unsafe int GetHostEntryForNameAsync(
string address,
AddressFamily family,
HostEntry* entry,
delegate* unmanaged<HostEntry*, int, void> callback);

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FreeHostEntry")]
internal static extern unsafe void FreeHostEntry(HostEntry* entry);
Expand Down
1 change: 1 addition & 0 deletions src/libraries/Native/Unix/Common/pal_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#cmakedefine01 HAVE_F_FULLFSYNC
#cmakedefine01 HAVE_O_CLOEXEC
#cmakedefine01 HAVE_GETIFADDRS
#cmakedefine01 HAVE_GETADDRINFO_A
#cmakedefine01 HAVE_UTSNAME_DOMAINNAME
#cmakedefine01 HAVE_STAT64
#cmakedefine01 HAVE_FORK
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/Native/Unix/System.Native/entrypoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ static const Entry s_sysNative[] =
DllImportEntry(SystemNative_ReadEvents)
DllImportEntry(SystemNative_CreateNetworkChangeListenerSocket)
DllImportEntry(SystemNative_CloseNetworkChangeListenerSocket)
DllImportEntry(SystemNative_PlatformSupportsGetAddrInfoAsync)
DllImportEntry(SystemNative_GetHostEntryForName)
DllImportEntry(SystemNative_GetHostEntryForNameAsync)
DllImportEntry(SystemNative_FreeHostEntry)
DllImportEntry(SystemNative_GetNameInfo)
DllImportEntry(SystemNative_GetDomainName)
Expand Down
4 changes: 4 additions & 0 deletions src/libraries/Native/Unix/System.Native/extra_libs.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ macro(append_extra_system_libs NativeLibsExtra)
if (CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS)
list(APPEND ${NativeLibsExtra} "-framework Foundation")
endif ()

if (CLR_CMAKE_TARGET_LINUX AND HAVE_GETADDRINFO_A)
VSadov marked this conversation as resolved.
Show resolved Hide resolved
list(APPEND ${NativeLibsExtra} anl)
endif ()
endmacro()
208 changes: 176 additions & 32 deletions src/libraries/Native/Unix/System.Native/pal_networking.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
#if HAVE_LINUX_CAN_H
#include <linux/can.h>
#endif
#if HAVE_GETADDRINFO_A
#include <signal.h>
#include <stdatomic.h>
#endif
#if HAVE_SYS_FILIO_H
#include <sys/filio.h>
#endif
Expand Down Expand Up @@ -335,37 +339,14 @@ static int32_t CopySockAddrToIPAddress(sockaddr* addr, sa_family_t family, IPAdd
return -1;
}

int32_t SystemNative_GetHostEntryForName(const uint8_t* address, int32_t addressFamily, HostEntry* entry)
static int32_t GetHostEntries(const uint8_t* address, struct addrinfo* info, HostEntry* entry)
{
if (address == NULL || entry == NULL)
{
return GetAddrInfoErrorFlags_EAI_BADARG;
}

int32_t ret = GetAddrInfoErrorFlags_EAI_SUCCESS;

struct addrinfo* info = NULL;
#if HAVE_GETIFADDRS
struct ifaddrs* addrs = NULL;
#endif

sa_family_t platformFamily;
if (!TryConvertAddressFamilyPalToPlatform(addressFamily, &platformFamily))
{
return GetAddrInfoErrorFlags_EAI_FAMILY;
}

struct addrinfo hint;
memset(&hint, 0, sizeof(struct addrinfo));
hint.ai_flags = AI_CANONNAME;
hint.ai_family = platformFamily;

int result = getaddrinfo((const char*)address, NULL, &hint, &info);
if (result != 0)
{
return ConvertGetAddrInfoAndGetNameInfoErrorsToPal(result);
}

entry->CanonicalName = NULL;
entry->Aliases = NULL;
entry->IPAddressList = NULL;
Expand Down Expand Up @@ -393,7 +374,8 @@ int32_t SystemNative_GetHostEntryForName(const uint8_t* address, int32_t address

#if HAVE_GETIFADDRS
char name[_POSIX_HOST_NAME_MAX];
result = gethostname((char*)name, _POSIX_HOST_NAME_MAX);

int result = gethostname((char*)name, _POSIX_HOST_NAME_MAX);

bool includeIPv4Loopback = true;
bool includeIPv6Loopback = true;
Expand Down Expand Up @@ -443,6 +425,8 @@ int32_t SystemNative_GetHostEntryForName(const uint8_t* address, int32_t address
}
}
}
#else
(void)address;
gfoidl marked this conversation as resolved.
Show resolved Hide resolved
#endif

if (entry->IPAddressCount > 0)
Expand Down Expand Up @@ -519,6 +503,166 @@ int32_t SystemNative_GetHostEntryForName(const uint8_t* address, int32_t address
return ret;
}

#if HAVE_GETADDRINFO_A
struct GetAddrInfoAsyncState
{
struct gaicb gai_request;
struct gaicb* gai_requests;
struct sigevent sigevent;

struct addrinfo hint;
HostEntry* entry;
GetHostEntryForNameCallback callback;
char address[];
};

static void GetHostEntryForNameAsyncComplete(sigval_t context)
{
struct GetAddrInfoAsyncState* state = (struct GetAddrInfoAsyncState*)context.sival_ptr;

atomic_thread_fence(memory_order_acquire);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the fence necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

This callback comes in on another thread. It ensures visibility to this thread of everything in state.

getaddrinfo_a probably does something like this already, so it might be safe to remove, but it isn't documented.


GetHostEntryForNameCallback callback = state->callback;

int ret = ConvertGetAddrInfoAndGetNameInfoErrorsToPal(gai_error(&state->gai_request));

if (ret == 0)
{
const uint8_t* address = (const uint8_t*)state->address;
struct addrinfo* info = state->gai_request.ar_result;

ret = GetHostEntries(address, info, state->entry);
}

assert(callback != NULL);
callback(state->entry, ret);

free(state);
}
#endif

static bool TrySetAddressFamily(int32_t addressFamily, struct addrinfo* hint)
{
sa_family_t platformFamily;
if (!TryConvertAddressFamilyPalToPlatform(addressFamily, &platformFamily))
{
return false;
}

memset(hint, 0, sizeof(struct addrinfo));

hint->ai_flags = AI_CANONNAME;
hint->ai_family = platformFamily;

return true;
}

int32_t SystemNative_PlatformSupportsGetAddrInfoAsync()
{
return HAVE_GETADDRINFO_A;
}

int32_t SystemNative_GetHostEntryForName(const uint8_t* address, int32_t addressFamily, HostEntry* entry)
{
if (address == NULL || entry == NULL)
{
return GetAddrInfoErrorFlags_EAI_BADARG;
}

struct addrinfo hint;
if (!TrySetAddressFamily(addressFamily, &hint))
{
return GetAddrInfoErrorFlags_EAI_FAMILY;
}

struct addrinfo* info = NULL;

int result = getaddrinfo((const char*)address, NULL, &hint, &info);
if (result != 0)
{
return ConvertGetAddrInfoAndGetNameInfoErrorsToPal(result);
}

return GetHostEntries(address, info, entry);
}

int32_t SystemNative_GetHostEntryForNameAsync(const uint8_t* address, int32_t addressFamily, HostEntry* entry, GetHostEntryForNameCallback callback)
{
#if HAVE_GETADDRINFO_A
if (address == NULL || entry == NULL)
{
return GetAddrInfoErrorFlags_EAI_BADARG;
wfurt marked this conversation as resolved.
Show resolved Hide resolved
}

size_t addrlen = strlen((const char*)address);

if (addrlen > _POSIX_HOST_NAME_MAX)
Copy link
Member Author

Choose a reason for hiding this comment

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

_POSIX_HOST_NAME_MAX is fixed to 255. It's more than the 253 according the RFC (cf. #34633 (comment)), but it matches

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW: neither the Windows-Pal nor the sync-version have a check for max hostname. Should these be added (at the managed side to catch them early)? Or leave it as the uncommon case and let the OS handle it?

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, it's better to check because you're computing the allocation length based on what is potentially user input; that could overflow and become an issue. By capping it to POSIX_HOST_NAME_MAX like you did here, the overflow is avoided (and there's no need to perform an overflow check).

In other cases, it's fine to leave it to the OS to verify, as they're passing the string as it is to the system calls/library functions.

{
return GetAddrInfoErrorFlags_EAI_BADARG;
}

sa_family_t platformFamily;
if (!TryConvertAddressFamilyPalToPlatform(addressFamily, &platformFamily))
{
return GetAddrInfoErrorFlags_EAI_FAMILY;
}

struct GetAddrInfoAsyncState* state = malloc(sizeof(*state) + addrlen + 1);

if (state == NULL)
{
return GetAddrInfoErrorFlags_EAI_MEMORY;
}

if (!TrySetAddressFamily(addressFamily, &state->hint))
{
free(state);
return GetAddrInfoErrorFlags_EAI_FAMILY;
}

memcpy(state->address, address, addrlen + 1);

*state = (struct GetAddrInfoAsyncState) {
.gai_request = {
.ar_name = state->address,
.ar_service = NULL,
.ar_request = &state->hint,
.ar_result = NULL
},
.gai_requests = &state->gai_request,
.sigevent = {
.sigev_notify = SIGEV_THREAD,
.sigev_value = {
.sival_ptr = state
},
.sigev_notify_function = GetHostEntryForNameAsyncComplete
},
.entry = entry,
.callback = callback
};

atomic_thread_fence(memory_order_release);

int32_t result = getaddrinfo_a(GAI_NOWAIT, &state->gai_requests, 1, &state->sigevent);

if (result != 0)
{
free(state);
return ConvertGetAddrInfoAndGetNameInfoErrorsToPal(result);
}

return result;
lpereira marked this conversation as resolved.
Show resolved Hide resolved
#else
(void)address;
(void)addressFamily;
(void)entry;
(void)callback;

// GetHostEntryForNameAsync is not supported on this platform.
return -1;
#endif
}

void SystemNative_FreeHostEntry(HostEntry* entry)
{
if (entry != NULL)
Expand Down Expand Up @@ -555,13 +699,13 @@ static inline NativeFlagsType ConvertGetNameInfoFlagsToNative(int32_t flags)
}

int32_t SystemNative_GetNameInfo(const uint8_t* address,
int32_t addressLength,
int8_t isIPv6,
uint8_t* host,
int32_t hostLength,
uint8_t* service,
int32_t serviceLength,
int32_t flags)
int32_t addressLength,
int8_t isIPv6,
uint8_t* host,
int32_t hostLength,
uint8_t* service,
int32_t serviceLength,
int32_t flags)
{
assert(address != NULL);
assert(addressLength > 0);
Expand Down
8 changes: 8 additions & 0 deletions src/libraries/Native/Unix/System.Native/pal_networking.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,16 @@ typedef struct
uint32_t Padding; // Pad out to 8-byte alignment
} SocketEvent;

PALEXPORT int32_t SystemNative_PlatformSupportsGetAddrInfoAsync(void);

PALEXPORT int32_t SystemNative_GetHostEntryForName(const uint8_t* address, int32_t addressFamily, HostEntry* entry);
gfoidl marked this conversation as resolved.
Show resolved Hide resolved

typedef void (*GetHostEntryForNameCallback)(HostEntry* entry, int status);
PALEXPORT int32_t SystemNative_GetHostEntryForNameAsync(const uint8_t* address,
int32_t addressFamily,
HostEntry* entry,
GetHostEntryForNameCallback callback);

PALEXPORT void SystemNative_FreeHostEntry(HostEntry* entry);


Expand Down
14 changes: 14 additions & 0 deletions src/libraries/Native/Unix/configure.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ include(CheckPrototypeDefinition)
include(CheckStructHasMember)
include(CheckSymbolExists)
include(CheckTypeSize)
include(CMakePushCheckState)

# CMP0075 Include file check macros honor CMAKE_REQUIRED_LIBRARIES.
if(POLICY CMP0075)
Expand Down Expand Up @@ -900,6 +901,19 @@ check_symbol_exists(
HAVE_INOTIFY_RM_WATCH)
set (CMAKE_REQUIRED_LIBRARIES ${PREVIOUS_CMAKE_REQUIRED_LIBRARIES})

if (CLR_CMAKE_TARGET_LINUX)
cmake_push_check_state(RESET)
set (CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE")
set (CMAKE_REQUIRED_LIBRARIES "-lanl")

check_symbol_exists(
getaddrinfo_a
netdb.h
HAVE_GETADDRINFO_A)

cmake_pop_check_state()
endif ()

set (HAVE_INOTIFY 0)
if (HAVE_INOTIFY_INIT AND HAVE_INOTIFY_ADD_WATCH AND HAVE_INOTIFY_RM_WATCH)
set (HAVE_INOTIFY 1)
Expand Down
Loading