[Android] Fix accessing network interfaces information #62780

simonrozsival · 2021-12-14T15:13:25Z

On (old) Android we need to load getifaddrs and freeifaddrs manually because the ifaddrs.h header is missing in older Android releases (before API 24) but the functions are availabe in libc.
I had to change the cmake configuration. check_symbol_exists wasn't reliable for wasm builds so I changed it to check_function_exists which we already use to detect getifaddrs in src/mono/cmake/configure.cmake

The code is based on the Xamarin Android implementation: https://github.com/xamarin/xamarin-android/blob/main/src/monodroid/jni/xamarin_getifaddrs.cc

This PR re-enables some tests that were disabled on Android in #50567
Ref #51303

ghost · 2021-12-14T15:13:32Z

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR is still WIP.

On (old) Android we need to load getifaddrs and freeifaddrs dynamically because the ifaddrs.h header is missing in older Android releases (before API 24).

The code is based on the Xamarin Android implementation: https://github.com/xamarin/xamarin-android/blob/main/src/monodroid/jni/xamarin_getifaddrs.cc

This PR re-enables some tests that were disabled on Android in #50567
Ref #51303

Author:	simonrozsival
Assignees:	-
Labels:	`area-System.Net`
Milestone:	-

simonrozsival · 2021-12-14T15:14:30Z

/azp run runtime-manual

azure-pipelines · 2021-12-14T15:14:43Z

Azure Pipelines successfully started running 1 pipeline(s).

simonrozsival · 2021-12-14T17:33:20Z

/azp run runtime-manual

azure-pipelines · 2021-12-14T17:33:35Z

Azure Pipelines successfully started running 1 pipeline(s).

simonrozsival · 2021-12-15T08:13:15Z

All the failing tests are unrelated to this PR.

simonrozsival · 2022-01-03T19:08:14Z

/azp run runtime-manual

azure-pipelines · 2022-01-03T19:08:28Z

Azure Pipelines successfully started running 1 pipeline(s).

This reverts commit f927aae.

This reverts commit 46d3ede.

simonrozsival · 2022-01-04T14:31:59Z

/azp run runtime-manual

azure-pipelines · 2022-01-04T14:32:16Z

Azure Pipelines successfully started running 1 pipeline(s).

…oid-get-network-interfaces-bug

simonrozsival · 2022-01-07T12:44:36Z

/azp run runtime-manual

azure-pipelines · 2022-01-07T12:44:50Z

Azure Pipelines successfully started running 1 pipeline(s).

jkotas · 2022-01-07T15:52:42Z

src/native/libs/System.Native/pal_interfaceaddresses.c

+    static atomic_bool initialized = false;
+    static atomic_bool access_guard = true;
+
+    while (!atomic_load(&initialized))


Is pthread_once available on Android? It would be better to use it here instead of the busy waiting loop.

If two threads happen to try to initialize this at the same time, one of them may end up busy waiting for multiple miliseconds here in this implementation that's not efficient. pthread_once does not have this problem.

pthread_once is available and it also uses atomic_load_explicit in a loop + futex syscalls, so it will most likely be heavier than this simple loop. The likelihood of two threads hitting this code at exactly the same time is so low that, IMO, it's not worth worrying about.

Yes, pthread_once has a loop similar to this one, except that it also handles the case that I have pointed out. The extra cost of pthread_once is negligible given what this whole method does.

The #1 rule for lock-free programming: Do not write lock-free algorithm yourselves unless you absolutely have to.

…oid-get-network-interfaces-bug

simonrozsival · 2022-01-10T16:17:04Z

/azp run runtime-manual

azure-pipelines · 2022-01-10T16:17:19Z

Azure Pipelines successfully started running 1 pipeline(s).

simonrozsival · 2022-01-11T14:17:59Z

The failing runtime-manual tests are unrelated to this PR.

simonrozsival · 2022-01-11T14:49:43Z

/backport to release/6.0-maui

github-actions · 2022-01-11T14:50:02Z

Started backporting to release/6.0-maui: https://github.com/dotnet/runtime/actions/runs/1683046431

github-actions · 2022-01-11T14:51:59Z

@simonrozsival backporting to release/6.0-maui failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Re-enable tests
Applying: Dynamically load getifaddrs if necessary
Using index info to reconstruct a base tree...
A	src/native/libs/System.Native/pal_interfaceaddresses.c
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/Native/Unix/System.Native/pal_interfaceaddresses.c
Applying: Prevent redeclaration of the ifaddrs struct
Using index info to reconstruct a base tree...
M	src/mono/cmake/config.h.in
A	src/native/libs/Common/pal_config.h.in
A	src/native/libs/System.Native/pal_interfaceaddresses.c
A	src/native/libs/configure.cmake
Falling back to patching base and 3-way merge...
Auto-merging src/mono/cmake/config.h.in
Auto-merging src/libraries/Native/Unix/configure.cmake
Auto-merging src/libraries/Native/Unix/System.Native/pal_interfaceaddresses.c
Auto-merging src/libraries/Native/Unix/Common/pal_config.h.in
Applying: Fix typo
Using index info to reconstruct a base tree...
A	src/native/libs/System.Native/pal_interfaceaddresses.c
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/Native/Unix/System.Native/pal_interfaceaddresses.c
Applying: Do not close the dynamic library
Using index info to reconstruct a base tree...
A	src/native/libs/System.Native/pal_interfaceaddresses.c
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/Native/Unix/System.Native/pal_interfaceaddresses.c
Applying: Enable the fixed functional tests in CI
error: sha1 information is lacking or useless (src/libraries/System.Net.NetworkInformation/tests/FunctionalTests/IPGlobalPropertiesTest.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0006 Enable the fixed functional tests in CI
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

* Re-enable tests * Dynamically load getifaddrs if necessary * Prevent redeclaration of the ifaddrs struct * Fix typo * Do not close the dynamic library * Enable the fixed functional tests in CI * Reduce code duplication for obtaining libc file name * Simplify usage of the function pointers * Move typedefs * Rename the _ensure_ function * Remove fptr typedefs * Update comment * Add missing include * Update comment * Remove unnecessary comment * Move static variable * Remove unnecessary change * Move LIBC_FILENAME to the utilities header * Avoid error if constant is undefined * Conditionally include ifaddrs * Try to fix cmake_symbol_exists issue for browser * Minor tweaks * Try different way of detecting getifaddrs * Use the hack only for Android builds * Revert "Move LIBC_FILENAME to the utilities header" This reverts commit 4e67687. * Revert "Reduce code duplication for obtaining libc file name" This reverts commit aca15d1. * Simplify opening libc * Update code style * Fix race condition * Prevent race condition * Switch locking implementation for a lock-free implementation * Enable unit test for Android * Try using weak symbols * Fix function name * Revert "Fix function name" This reverts commit f927aae. * Revert "Try using weak symbols" This reverts commit 46d3ede. * Refactor code to use pthread_once

…tion (#63628) * [Android] Fix accessing network interfaces information (#62780) * Re-enable tests * Dynamically load getifaddrs if necessary * Prevent redeclaration of the ifaddrs struct * Fix typo * Do not close the dynamic library * Enable the fixed functional tests in CI * Reduce code duplication for obtaining libc file name * Simplify usage of the function pointers * Move typedefs * Rename the _ensure_ function * Remove fptr typedefs * Update comment * Add missing include * Update comment * Remove unnecessary comment * Move static variable * Remove unnecessary change * Move LIBC_FILENAME to the utilities header * Avoid error if constant is undefined * Conditionally include ifaddrs * Try to fix cmake_symbol_exists issue for browser * Minor tweaks * Try different way of detecting getifaddrs * Use the hack only for Android builds * Revert "Move LIBC_FILENAME to the utilities header" This reverts commit 4e67687. * Revert "Reduce code duplication for obtaining libc file name" This reverts commit aca15d1. * Simplify opening libc * Update code style * Fix race condition * Prevent race condition * Switch locking implementation for a lock-free implementation * Enable unit test for Android * Try using weak symbols * Fix function name * Revert "Fix function name" This reverts commit f927aae. * Revert "Try using weak symbols" This reverts commit 46d3ede. * Refactor code to use pthread_once * [Android] Throw PNSE for unavailable network information (#63633) * Update tests * Add android specific implementation * Add UnsupportedOSPlatform attributes * Fix typo * Remove unnecessary file reference * Clean-up code * Minor code clean-up * Remove dictionary * Refactoring * Revert comment change * Fix usage of Interop.Sys.GetNetworkInterfaces

simonrozsival added 5 commits December 14, 2021 15:09

Re-enable tests

bd5f8d0

Dynamically load getifaddrs if necessary

56e69e7

Prevent redeclaration of the ifaddrs struct

0841d50

Fix typo

863d47b

Do not close the dynamic library

51e3a39

ghost added the area-System.Net label Dec 14, 2021

Enable the fixed functional tests in CI

88044b9

simonrozsival added 17 commits December 15, 2021 10:06

Reduce code duplication for obtaining libc file name

aca15d1

Simplify usage of the function pointers

f61a115

Move typedefs

6e1f1f2

Rename the _ensure_ function

1a6babd

Remove fptr typedefs

05a8e64

Update comment

332d1fb

Add missing include

1b5edd9

Update comment

ef0a9aa

Remove unnecessary comment

a4b340c

Move static variable

e9b04ee

Remove unnecessary change

d0b040c

Move LIBC_FILENAME to the utilities header

4e67687

Avoid error if constant is undefined

8a1ef53

Conditionally include ifaddrs

f90422d

Try to fix cmake_symbol_exists issue for browser

be71333

Minor tweaks

99ec5ed

Try different way of detecting getifaddrs

a403fc2

simonrozsival added 3 commits January 3, 2022 17:46

Enable unit test for Android

4482fe5

Try using weak symbols

46d3ede

Fix function name

f927aae

simonrozsival added 2 commits January 4, 2022 15:30

Revert "Fix function name"

7e0ad11

This reverts commit f927aae.

Revert "Try using weak symbols"

3dd003e

This reverts commit 46d3ede.

Merge branch 'main' of github.com:simonrozsival/runtime into fix-andr…

286f0d1

…oid-get-network-interfaces-bug

jkotas reviewed Jan 7, 2022

View reviewed changes

grendello approved these changes Jan 10, 2022

View reviewed changes

simonrozsival added 2 commits January 10, 2022 17:14

Refactor code to use pthread_once

10b81dc

Merge branch 'main' of github.com:simonrozsival/runtime into fix-andr…

bec5f22

…oid-get-network-interfaces-bug

jkotas approved these changes Jan 10, 2022

View reviewed changes

simonrozsival merged commit 8c47d32 into dotnet:main Jan 11, 2022

karelz added this to the 7.0.0 milestone Jan 11, 2022

This was referenced Jan 11, 2022

[release/6.0-maui] [Android] Fix accessing network interfaces information #63628

Merged

[Android] Throw PNSE for unavailable network information #63633

Merged

ghost locked as resolved and limited conversation to collaborators Feb 10, 2022

[Android] Fix accessing network interfaces information #62780

[Android] Fix accessing network interfaces information #62780

Uh oh!

Conversation

simonrozsival commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Dec 14, 2021

Uh oh!

simonrozsival commented Dec 14, 2021

Uh oh!

azure-pipelines bot commented Dec 14, 2021

Uh oh!

simonrozsival commented Dec 14, 2021

Uh oh!

azure-pipelines bot commented Dec 14, 2021

Uh oh!

simonrozsival commented Dec 15, 2021

Uh oh!

simonrozsival commented Jan 3, 2022

Uh oh!

azure-pipelines bot commented Jan 3, 2022

Uh oh!

simonrozsival commented Jan 4, 2022

Uh oh!

azure-pipelines bot commented Jan 4, 2022

Uh oh!

simonrozsival commented Jan 7, 2022

Uh oh!

azure-pipelines bot commented Jan 7, 2022

Uh oh!

jkotas Jan 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grendello Jan 10, 2022

Choose a reason for hiding this comment

Uh oh!

jkotas Jan 10, 2022

Choose a reason for hiding this comment

Uh oh!

simonrozsival commented Jan 10, 2022

Uh oh!

azure-pipelines bot commented Jan 10, 2022

Uh oh!

simonrozsival commented Jan 11, 2022

Uh oh!

simonrozsival commented Jan 11, 2022

Uh oh!

github-actions bot commented Jan 11, 2022

Uh oh!

github-actions bot commented Jan 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

simonrozsival commented Dec 14, 2021 •

edited

Loading

jkotas Jan 7, 2022 •

edited

Loading