[PATCH v4 0/1] MR256: ntdll: Do not open directory file when setting the same directory path.

[Paul Gofman (＠gofman)]

Fixes The Rising of The Shield Hero: Relieve the Animation hangs on start (and maybe something else with ENIGMA protector).

The protector implements virtual file system which hotpatches a lot of file related functions, in particular, NtOpenFile(). When NtOpenFile() is called from RtlSetCurrentDirectory_U() with peb lock held the hotpatcher takes its own lock and that can deadlock with other threads performing file / path operations ending up in get_full_path_helper() which also takes PEB lock. Once the hotpatcher is initialized the game only sets "." or the same full path as current and may do that rather often.

It seems that reopening the file handle for the same directory is redundant anyway, so probably makes sense to just avoid that part.

--
  v4: ntdll: Do not open directory file when setting the same directory path.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/256

[PATCH v4 1/1] ntdll: Do not open directory file when setting the same directory path.

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com>
---
 dlls/kernel32/tests/path.c | 18 ++++++++++++++----
 dlls/ntdll/path.c          | 22 +++++++++++++++++-----
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/dlls/kernel32/tests/path.c b/dlls/kernel32/tests/path.c
index 3c57e81f4c6..eb2c551dcca 100644
--- a/dlls/kernel32/tests/path.c
+++ b/dlls/kernel32/tests/path.c
@@ -450,6 +450,8 @@ static void test_CurrentDirectoryA(CHAR *origdir, CHAR *newdir)
   CHAR tmpstr[MAX_PATH],tmpstr1[MAX_PATH];
   char *buffer;
   DWORD len,len1;
+  HANDLE handle;
+  BOOL ret;
 /* Save the original directory, so that we can return to it at the end
    of the test
 */
@@ -505,10 +507,18 @@ static void test_CurrentDirectoryA(CHAR *origdir, CHAR *newdir)
   sprintf(tmpstr,"%s\\",newdir);
   test_setdir(origdir,tmpstr,newdir,1,"check 1");
   test_setdir(origdir,newdir,NULL,1,"check 2");
-/* Set the directory to the working area.  We just tested that this works,
-   so why check it again.
-*/
-  SetCurrentDirectoryA(newdir);
+
+/* Check that SetCurrentDirectory doesn't reopen directory file when setting the same directory. */
+  handle = NtCurrentTeb()->Peb->ProcessParameters->CurrentDirectory.Handle;
+  ret = SetCurrentDirectoryA(newdir);
+  ok(ret, "SetCurrentDirectoryA failed.\n");
+  ok(handle != NtCurrentTeb()->Peb->ProcessParameters->CurrentDirectory.Handle, "got same handle.\n");
+  handle = NtCurrentTeb()->Peb->ProcessParameters->CurrentDirectory.Handle;
+
+  ret = SetCurrentDirectoryA(newdir);
+  ok(ret, "SetCurrentDirectoryA failed.\n");
+  ok(handle == NtCurrentTeb()->Peb->ProcessParameters->CurrentDirectory.Handle, "got different handle.\n");
+
 /* Check that SetCurrentDirectory fails when a nonexistent dir is specified */
   sprintf(tmpstr,"%s\\%s\\%s",newdir,SHORTDIR,NONDIR_SHORT);
   test_setdir(newdir,tmpstr,NULL,0,"check 3");
diff --git a/dlls/ntdll/path.c b/dlls/ntdll/path.c
index cccd000a6c6..889d020bd8e 100644
--- a/dlls/ntdll/path.c
+++ b/dlls/ntdll/path.c
@@ -905,13 +905,13 @@ ULONG WINAPI RtlGetCurrentDirectory_U(ULONG buflen, LPWSTR buf)
 NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir)
 {
     FILE_FS_DEVICE_INFORMATION device_info;
+    ULONG size, compare_size;
     OBJECT_ATTRIBUTES attr;
     UNICODE_STRING newdir;
     IO_STATUS_BLOCK io;
     CURDIR *curdir;
     HANDLE handle;
     NTSTATUS nts;
-    ULONG size;
     PWSTR ptr;
 
     newdir.Buffer = NULL;
@@ -929,6 +929,22 @@ NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir)
         goto out;
     }
 
+    size = newdir.Length / sizeof(WCHAR);
+    ptr = newdir.Buffer;
+    ptr += 4;  /* skip \??\ prefix */
+    size -= 4;
+
+    if (size && ptr[size - 1] == '\\') compare_size = size - 1;
+    else                               compare_size = size;
+
+    if (curdir->DosPath.Length == (compare_size + 1) * sizeof(WCHAR)
+        && !wcsnicmp( curdir->DosPath.Buffer, ptr, compare_size ))
+    {
+        TRACE( "dir %s is the same as current.\n", debugstr_us(dir) );
+        nts = STATUS_SUCCESS;
+        goto out;
+    }
+
     attr.Length = sizeof(attr);
     attr.RootDirectory = 0;
     attr.Attributes = OBJ_CASE_INSENSITIVE;
@@ -953,10 +969,6 @@ NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir)
     curdir->Handle = handle;
 
     /* append trailing \ if missing */
-    size = newdir.Length / sizeof(WCHAR);
-    ptr = newdir.Buffer;
-    ptr += 4;  /* skip \??\ prefix */
-    size -= 4;
     if (size && ptr[size - 1] != '\\') ptr[size++] = '\\';
 
     /* convert \??\UNC\ path to \\ prefix */
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/256

[Paul Gofman (＠gofman)]

v4:
    - Add test which suggests that Windows does not reopen the directory file when setting the same directory over.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/256#note_2971