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

Missing events on Windows 10 (Performance issues) #1019

Open
BoshenGuan opened this issue Oct 24, 2023 · 9 comments
Open

Missing events on Windows 10 (Performance issues) #1019

BoshenGuan opened this issue Oct 24, 2023 · 9 comments

Comments

@BoshenGuan
Copy link

BoshenGuan commented Oct 24, 2023

ISSUE-1: event_buffer = ctypes.create_string_buffer(BUFFER_SIZE) creates a big buffer every time in the loop. That is a bad idea. It would be better to create one and re-use on the following calls.

@BoshenGuan BoshenGuan changed the title pywin32 cause missing events on Windows 10 Missing events on Windows 10 (Performance issues) Oct 24, 2023
@BoshenGuan
Copy link
Author

Other performance/code issues:
ISSUE-2: ctypes.byref(nbytes) and return event_buffer.raw, int(nbytes.value) is ignored when nbytes is 0. When nbytes is 0, it means buffer-overflow. The code does not deal with 0 nbytes properly, which can lead to missing events.

@BoshenGuan
Copy link
Author

BoshenGuan commented Oct 24, 2023

Other performance/code issues:
ISSUE-3: Although not in use, the OVERLAPPED structure is wrong. There is a union in C/C++ header for OVERLAPPED structure. Therefore,

class OVERLAPPED(ctypes.Structure):
    _fields_ = [
        ("Internal", LPVOID),
        ("InternalHigh", LPVOID),
        ("Offset", ctypes.wintypes.DWORD),
        ("OffsetHigh", ctypes.wintypes.DWORD),
        ("Pointer", LPVOID),
        ("hEvent", ctypes.wintypes.HANDLE),
    ]

shoud be

class OVERLAPPED(ctypes.Structure):
    _fields_ = [
        ("Internal", ctypes.wintypes.LPVOID),
        ("InternalHigh", ctypes.wintypes.LPVOID),
        ("Offset", ctypes.wintypes.DWORD),
        ("OffsetHigh", ctypes.wintypes.DWORD),
        ("hEvent", ctypes.wintypes.HANDLE),
    ]

. Because Pointer is in the union.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Oct 24, 2023

Wow, you seem to have a good knowledge of the win API.
Would you mind creating PRs for each issue 🙏🏻 (if you can add tests too to prevent regressions, that would be awesome too)?

@BoboTiG
Copy link
Collaborator

BoboTiG commented Oct 24, 2023

ISSUE-3: it is used here:

ctypes.POINTER(OVERLAPPED), # lpOverlapped

@matkuki
Copy link

matkuki commented Nov 25, 2023

@BoshenGuan
Can you give more details on how to fix ISSUE-2?
I'm getting lots of missing events with moving large numbers of items into a directory.

P.S.:
Found it: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-readdirectorychangesw#remarks

@matkuki
Copy link

matkuki commented Nov 25, 2023

@BoshenGuan
I still don't know how to handle a nbytes == 0 situation. I have tried increasing the BUFFER_SIZE, it makes it a bit more consistant, but still there are missed events!
I'm not sure what the documentation means in the last sentence:

Upon successful synchronous completion, the lpBuffer parameter is a formatted buffer and the number of bytes written to the buffer is available in lpBytesReturned. If the number of bytes transferred is zero, the buffer was either too large for the system to allocate or too small to provide detailed information on all the changes that occurred in the directory or subtree. In this case, you should compute the changes by enumerating the directory or subtree.

In this case, you should compute the changes by enumerating the directory or subtree.: Does that mean a manual walk throught the directory is needed in this case?

@matkuki
Copy link

matkuki commented Nov 27, 2023

Here is a very detailed answer of why there is no escaping missing events:
https://stackoverflow.com/a/49888600/2603918

@matkuki
Copy link

matkuki commented Nov 28, 2023

@BoboTiG @BoshenGuan @tommorris @pilt
There seems to be no way to ensure catching all the events, especially not in Python. In the case that the ReadDirectoryChangesW returns nbytes == 0, how can we signal this to the user that a rescan is necessary?

@matkuki
Copy link

matkuki commented Dec 26, 2023

Python 3.12 solves this issue on my machine without any changes in the code! No idea why currently.

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

No branches or pull requests

3 participants