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

Switching out undocumented NtQueryObject for GetFileInformationByHandleEx #2377

Closed
wants to merge 1 commit into from
Closed

Switching out undocumented NtQueryObject for GetFileInformationByHandleEx #2377

wants to merge 1 commit into from

Conversation

assarbad
Copy link

  • Switching out undocumented (NT native) function NtQueryObject() for
    the documented Win32 alternative GetFileInformationByHandleEx().
  • Does not fix the stalls with git push for git://-protocol servers
    on Windows, but it's an improvement nevertheless because it avoids
    an undocumented function in favor for a documented one.

This is mostly meant to draw the attention to what I've tried so far ... unfortunately the underlying issue persists even with this change.

xref #2376

- Switching out undocumented (NT native) function NtQueryObject() for
  the documented Win32 alternative GetFileInformationByHandleEx().
- Does not fix the stalls with 'git push' for git://-protocol servers
  on Windows, but it's an improvement nevertheless because it avoids
  an undocumented function in favor for a documented one.
@dscho
Copy link
Member

dscho commented May 13, 2022

@assarbad what's your take on this PR, should it be left open, still?

@assarbad
Copy link
Author

If you like, I can update it to match the current state of the code. I haven't looked at it in at least 2 years.

Otherwise, if you don't find it desirable to use the official Win32 API, that's also fine with me and it can be closed.

@dscho
Copy link
Member

dscho commented May 16, 2022

Wasn't the original idea to improve performance? If so, I suspect that the (undocumented) call provides better performance.

@dscho
Copy link
Member

dscho commented Feb 6, 2023

@assarbad are you planning on working on this any further?

@assarbad
Copy link
Author

assarbad commented Feb 8, 2023

Wasn't the original idea to improve performance? If so, I suspect that the (undocumented) call provides better performance.

Technically, that may even be the case. But again, it'd be the documented alternative on which you can rely going forward (and it's a fairly shallow wrapper around the native function in this case, unlike some other Win32 API functions).

@assarbad are you planning on working on this any further?

@dscho Since you seem to prefer the existing use of the NT native function, I guess there's nothing left to do here.

It doesn't resolve the blocking pipes issue #2376 either way 😑

@assarbad assarbad closed this Feb 8, 2023
@dscho
Copy link
Member

dscho commented Feb 9, 2023

Thank you for explaining your thinking, and I agree that we can leave this closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants