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.

Sign up for GitHub

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

Fix Android device issues related to filesystem/IO #58586

Merged
merged 3 commits into from
Sep 3, 2021
Merged

Fix Android device issues related to filesystem/IO #58586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db5cf28
Fix Android device issues related to filesystem/IO
akoeplinger Sep 2, 2021
7067011
PR feedback
akoeplinger Sep 2, 2021
a699d1e
Reenable Net5Compat project
akoeplinger Sep 3, 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
1 change: 1 addition & 0 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.FormatInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ private static DriveType GetDriveType(string fileSystemName)
// This list is based primarily on "man fs", "man mount", "mntent.h", "/proc/filesystems", coreutils "stat.c",
// and "wiki.debian.org/FileSystem". It can be extended over time as we find additional file systems that should
// be recognized as a particular drive type.
// Keep this in sync with the UnixFileSystemTypes enum in Interop.UnixFileSystemTypes.cs
switch (fileSystemName)
{
case "cddafs":
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ internal enum UnixFileSystemTypes : long
befs = 0x42465331,
bdevfs = 0x62646576,
bfs = 0x1BADFACE,
bpf_fs = 0xCAFE4A11,
binfmt_misc = 0x42494E4D,
bootfs = 0xA56D3FF9,
btrfs = 0x9123683E,
Expand All @@ -53,6 +54,7 @@ internal enum UnixFileSystemTypes : long
ext2 = 0xEF53,
ext3 = 0xEF53,
ext4 = 0xEF53,
f2fs = 0xF2F52010,
fat = 0x4006,
fd = 0xF00D1E,
fhgfs = 0x19830326,
Expand Down Expand Up @@ -122,6 +124,7 @@ internal enum UnixFileSystemTypes : long
sysv2 = 0x012FF7B6,
sysv4 = 0x012FF7B5,
tmpfs = 0x01021994,
tracefs = 0x74726163,
ubifs = 0x24051905,
udf = 0x15013346,
ufs = 0x00011954,
Expand Down
13 changes: 12 additions & 1 deletion src/libraries/Native/Unix/System.Native/pal_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1427,6 +1427,7 @@ int64_t SystemNative_GetFileSystemType(intptr_t fd)
else if (strcmp(statfsArgs.f_basetype, "befs") == 0) result = 0x42465331;
else if (strcmp(statfsArgs.f_basetype, "bdevfs") == 0) result = 0x62646576;
else if (strcmp(statfsArgs.f_basetype, "bfs") == 0) result = 0x1BADFACE;
else if (strcmp(statfsArgs.f_basetype, "bpf_fs") == 0) result = 0xCAFE4A11;
else if (strcmp(statfsArgs.f_basetype, "binfmt_misc") == 0) result = 0x42494E4D;
else if (strcmp(statfsArgs.f_basetype, "bootfs") == 0) result = 0xA56D3FF9;
else if (strcmp(statfsArgs.f_basetype, "btrfs") == 0) result = 0x9123683E;
Expand All @@ -1452,6 +1453,7 @@ int64_t SystemNative_GetFileSystemType(intptr_t fd)
else if (strcmp(statfsArgs.f_basetype, "ext2") == 0) result = 0xEF53;
else if (strcmp(statfsArgs.f_basetype, "ext3") == 0) result = 0xEF53;
else if (strcmp(statfsArgs.f_basetype, "ext4") == 0) result = 0xEF53;
else if (strcmp(statfsArgs.f_basetype, "f2fs") == 0) result = 0xF2F52010;
else if (strcmp(statfsArgs.f_basetype, "fat") == 0) result = 0x4006;
else if (strcmp(statfsArgs.f_basetype, "fd") == 0) result = 0xF00D1E;
else if (strcmp(statfsArgs.f_basetype, "fhgfs") == 0) result = 0x19830326;
Expand Down Expand Up @@ -1519,6 +1521,7 @@ int64_t SystemNative_GetFileSystemType(intptr_t fd)
else if (strcmp(statfsArgs.f_basetype, "sysv2") == 0) result = 0x012FF7B6;
else if (strcmp(statfsArgs.f_basetype, "sysv4") == 0) result = 0x012FF7B5;
else if (strcmp(statfsArgs.f_basetype, "tmpfs") == 0) result = 0x01021994;
else if (strcmp(statfsArgs.f_basetype, "tracefs") == 0) result = 0x74726163;
else if (strcmp(statfsArgs.f_basetype, "ubifs") == 0) result = 0x24051905;
else if (strcmp(statfsArgs.f_basetype, "udf") == 0) result = 0x15013346;
else if (strcmp(statfsArgs.f_basetype, "ufs") == 0) result = 0x00011954;
Expand Down Expand Up @@ -1560,13 +1563,21 @@ int32_t SystemNative_LockFileRegion(intptr_t fd, int64_t offset, int64_t length,
struct flock lockArgs;
#endif

#if defined(TARGET_ANDROID) && defined(HAVE_FLOCK64)
// On Android, fcntl is always implemented by fcntl64 but before https://github.com/aosp-mirror/platform_bionic/commit/09e77f35ab8d291bf88302bb9673aaa518c6bcb0
// there was no remapping of F_SETLK to F_SETLK64 when _FILE_OFFSET_BITS=64 (which we set in eng/native/configurecompiler.cmake) so we need to always pass F_SETLK64
int command = F_SETLK64;
#else
int command = F_SETLK;
#endif
Comment on lines +1566 to +1572
Copy link
Member

@am11 am11 Sep 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Is there a case where generic condition won't work:

Suggested change
#if defined(TARGET_ANDROID) && defined(HAVE_FLOCK64)
// On Android, fcntl is always implemented by fcntl64 but before https://github.com/aosp-mirror/platform_bionic/commit/09e77f35ab8d291bf88302bb9673aaa518c6bcb0
// there was no remapping of F_SETLK to F_SETLK64 when _FILE_OFFSET_BITS=64 (which we set in eng/native/configurecompiler.cmake) so we need to always pass F_SETLK64
int command = F_SETLK64;
#else
int command = F_SETLK;
#endif
#ifdef F_SETLK64
int command = F_SETLK64;
#else
int command = F_SETLK;
#endif

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure and since I want the change to go back to release/6.0 I thought it's better to keep the existing code as is. We can try your version for 7.0.


lockArgs.l_type = unixLockType;
lockArgs.l_whence = SEEK_SET;
lockArgs.l_start = (off_t)offset;
lockArgs.l_len = (off_t)length;

int32_t ret;
while ((ret = fcntl (ToFileDescriptor(fd), F_SETLK, &lockArgs)) < 0 && errno == EINTR);
while ((ret = fcntl (ToFileDescriptor(fd), command, &lockArgs)) < 0 && errno == EINTR);
return ret;
}

Expand Down
3 changes: 1 addition & 2 deletions src/libraries/System.IO/tests/Net5CompatTests/System.IO.Net5Compat.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<TestRuntime>true</TestRuntime>
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
<!-- Windows is currently the only OS for which we provide a new strategy, so we test the Net5Compat only for Windows -->
<TargetFrameworks>$(NetCoreAppCurrent)-windows</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<Compile Include="..\**\*.cs" />
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Ping/tests/FunctionalTests/UnixPingUtilityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ public static void TimeoutIsRespected(int timeout)
p.Start();
p.WaitForExit();

//ensure that the process takes longer than or equal to 'timeout'
Assert.True(stopWatch.ElapsedMilliseconds >= timeout);
//ensure that the process takes longer than or within 10ms of 'timeout', with a 5s maximum
Assert.InRange(stopWatch.ElapsedMilliseconds, timeout - 10, 5000);
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
Expand Down