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

memory leak doubt and thread conflict #174

Open
hgyxbll opened this issue Mar 27, 2024 · 1 comment
Open

memory leak doubt and thread conflict #174

hgyxbll opened this issue Mar 27, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@hgyxbll
Copy link

hgyxbll commented Mar 27, 2024

memory leak doubt:
at WatcherWin32.cpp.
CreateWatch use below code.

/// Starts monitoring a directory.
WatcherStructWin32* CreateWatch( LPCWSTR szDirectory, bool recursive, DWORD NotifyFilter,
								 HANDLE iocp ) {
	WatcherStructWin32* tWatch;
	size_t ptrsize = sizeof( *tWatch );
	tWatch = static_cast<WatcherStructWin32*>(
		HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, ptrsize ) );   // there malloc 

DestroyWatch use below code.

void DestroyWatch( WatcherStructWin32* pWatch ) {
	if ( pWatch ) {
		WatcherWin32* tWatch = pWatch->Watch;
		tWatch->StopNow = true;
		CancelIoEx( pWatch->Watch->DirHandle, &pWatch->Overlapped );
		CloseHandle( pWatch->Watch->DirHandle );
		efSAFE_DELETE_ARRAY( pWatch->Watch->DirName );
		efSAFE_DELETE( pWatch->Watch );
	}
}

this do not HeapFree pWatch.

Is it may leak memory?

Thread conflict:
at FileWatcherWin32.cpp.

void FileWatcherWin32::run() {
	do {
		if ( mInitOK && !mWatches.empty() ) {
			DWORD numOfBytes = 0;
			OVERLAPPED* ov = NULL;
			ULONG_PTR compKey = 0;
			BOOL res = FALSE;

			while ( ( res = GetQueuedCompletionStatus( mIOCP, &numOfBytes, &compKey, &ov,
													   INFINITE ) ) != FALSE ) {
				if ( compKey != 0 && compKey == reinterpret_cast<ULONG_PTR>( this ) ) {
					break;
				} else {
					Lock lock( mWatchesLock );
					WatchCallback( numOfBytes, ov );
				}
			}

if GetQueuedCompletionStatus is ok, and than will call Lock lock( mWatchesLock );.
but before call it, user call removeWatch, then WatchCallback will access null pointer.

@SpartanJ
Copy link
Owner

this do not HeapFree pWatch.

I think you're right. Sorry, this is very old code, I should review it again.

if GetQueuedCompletionStatus is ok, and than will call Lock lock( mWatchesLock );.
 but before call it, user call removeWatch, then WatchCallback will access null pointer.

You're correct, after the lock it should check that the overlapped object is still available.

I'll fix them next week since I don't have access to a Windows machine right now and I would like to test the fix.

Thanks for taking a look at the implementation!

@SpartanJ SpartanJ self-assigned this Mar 27, 2024
@SpartanJ SpartanJ added the bug Something isn't working label Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants