efsw icon indicating copy to clipboard operation
efsw copied to clipboard

memory leak doubt and thread conflict

Open hgyxbll opened this issue 11 months ago • 1 comments

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.

hgyxbll avatar Mar 27 '24 10:03 hgyxbll

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 avatar Mar 27 '24 14:03 SpartanJ

Memory lean fixed here. And possible incorrect memory access fixed here.

SpartanJ avatar Aug 04 '24 04:08 SpartanJ