Skip to content

Commit

Permalink
NameResolutionPal.Unix enabled async name resolution (dotnet#34633)
Browse files Browse the repository at this point in the history
* Native implementation

* Native tests

* Managed implementation

* Managed tests

* Revert Interop.GetHostName change -- it's needed at other places too

So this change was a bad idea ;-)

* Fixed builds failures due to "unused parameter"

* Native: move struct addrinfo hint from stack-alloc to heap-alloc

Before I didn't last the async operation as it was on the stack. Now it's part of the GetAddrInfoAsyncState which gets heap allocted, so it last.
I'm not sure if addrinfo needs to last the async operation, as the native tests pass either way. This change makes it more correct, nevertheless.

* PR feedback

* Fixed leak in tests.c according to valgrind's run

* Fixed bug due to marshalled string argument

* More managed PalTests

* Revert "Native tests"

This reverts commit 8c955bc and d7cb2be.

* Handle the case of HostName="" and tests for this

* Use flexible array member to hold the address in struct state

Cf. dotnet#34633 (comment)

* Nits in native layer

Cf. dotnet#34633 (comment)

* Fixed native merge conflict + added AddressFamily-handling

* Fixed managed merge conflicts + added AddressFamily-handling

* Updated NameResolutionPalTests for AddressFamily

* Removed EnsureSocketsAreInitialized

On Unix this was a nop anyway, dotnet#43284 removed it from Windows.

* Fixed bug at native side

Seems like "hint" must last the async operation, so stack-only won't do it.
Tests will crash with
```
The active test run was aborted. Reason: Test host process crashed : .../runtime/src/libraries/Native/Unix/System.Native/pal_networking.c (315): error -7: Unknown AddrInfo error flag. Unknown error -7 (0 failed)
```

If put into the heap, it succeeds.

* Refactor setting of the address family into TrySetAddressFamily

* Fixed unused parameter warning / failure

* Little cleanup

* Fixed (unrelated) test failures

Cf. dotnet#34633 (comment)

* Made LoggingTest async instead of GetAwaiter().GetResult()

* Use function pointer

* Use OperatinngSystem instead of RuntimeInformation in tests

* PR Feedback for CMake

* Update src/libraries/Native/Unix/System.Native/extra_libs.cmake

Co-authored-by: Günther Foidl <gue@korporal.at>

* Revert "Update src/libraries/Native/Unix/System.Native/extra_libs.cmake"

This reverts commit fa9c6f9.

* Another to build single file host

Based on dotnet#34633 (comment)

* Test for !Windows instead excluding several Unix-flavors

Cf. dotnet#34633 (comment)

Co-authored-by: Vladimir Sadov <vsadov@microsoft.com>
  • Loading branch information
gfoidl and VSadov committed Jan 14, 2021
1 parent 82a2cda commit f4ae905
Show file tree
Hide file tree
Showing 12 changed files with 723 additions and 129 deletions.
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)
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;
#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);

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;
}

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

if (addrlen > _POSIX_HOST_NAME_MAX)
{
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;
#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);

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

0 comments on commit f4ae905

Please sign in to comment.