signalEvent not be closed

wenhaochen · November 23, 2018, 7:25am

Hi，

code from https://github.com/elastic/beats/blob/master/winlogbeat/eventlog/wineventlog.go#L125
signalEvent not be closed, handle leak might happen when calling Open()func many times ?
and one more thing confuse me:
Pull Subscriptions example as below( https://docs.microsoft.com/zh-cn/windows/desktop/WES/subscribing-to-events)

 <... more ...>
 aWaitHandles[1] = CreateEvent(NULL, TRUE, TRUE, NULL);
 <... more ...>

    // Subscribe to events.
    hSubscription = EvtSubscribe(NULL, aWaitHandles[1], pwsPath, pwsQuery, NULL, NULL, NULL, EvtSubscribeStartAtOldestRecord);

// Loop until the user presses a key or there is an error.
while (true)
{
    dwWait = WaitForMultipleObjects(sizeof(aWaitHandles)/sizeof(HANDLE), aWaitHandles, FALSE, INFINITE);

    if (0 == dwWait - WAIT_OBJECT_0)  // Console input
    {
        if (IsKeyEvent(aWaitHandles[0]))
            break;
    }
    else if (1 == dwWait - WAIT_OBJECT_0) // Query results
    {
        if (ERROR_NO_MORE_ITEMS != (status = EnumerateResults(hSubscription)))
        {
            break;
        }

        ResetEvent(aWaitHandles[1]);
    }
<... more ... >
}

compared to winlogbeat codes

github.com

elastic/beats/blob/master/winlogbeat/eventlog/wineventlog.go#L125~L140


	} else {
		bookmark, err = win.CreateBookmarkFromRecordID(l.channelName, state.RecordNumber)
	}
	if err != nil {
		return err
	}
	defer win.Close(bookmark)


	// Using a pull subscription to receive events. See:
	// https://msdn.microsoft.com/en-us/library/windows/desktop/aa385771(v=vs.85).aspx#pull
	signalEvent, err := windows.CreateEvent(nil, 0, 0, nil)
	if err != nil {
		return nil
	}


	debugf("%s using subscription query=%s", l.logPrefix, l.query)
	subscriptionHandle, err := win.Subscribe(
		0, // Session - nil for localhost
		signalEvent,
		"",       // Channel - empty b/c channel is in the query
		l.query,  // Query - nil means all events

seems we do not use signalEvent as aWaitHandles[1]) do

thanks

andrewkroh · November 25, 2018, 10:56pm

I think Open() is only called once for each event log so we haven’t observed any leaks. But it does look like the Close() method should be updated to all close the handle returned by CreateEvent.

wenhaochen · November 28, 2018, 2:44am

In my case, if error happens, I'll call Open()to recreate new eventlog instance , then handle leaks observed when some error in my env;

It does hard to lead to leaks if called once, but still might cause issue sometime
do u think should I post any fix code?

thanks

andrewkroh · November 28, 2018, 4:07am

We might as well fix it, so yes, please.

wenhaochen · November 30, 2018, 3:01am

one more:

github.com

elastic/beats/blob/master/winlogbeat/eventlog/wineventlog.go#L126~L128


		bookmark, err = win.CreateBookmarkFromRecordID(l.channelName, state.RecordNumber)
	}
	if err != nil {
		return err
	}
	defer win.Close(bookmark)


	// Using a pull subscription to receive events. See:
	// https://msdn.microsoft.com/en-us/library/windows/desktop/aa385771(v=vs.85).aspx#pull
	signalEvent, err := windows.CreateEvent(nil, 0, 0, nil)
	if err != nil {
		return nil
	}


	debugf("%s using subscription query=%s", l.logPrefix, l.query)
	subscriptionHandle, err := win.Subscribe(
		0, // Session - nil for localhost
		signalEvent,
		"",       // Channel - empty b/c channel is in the query
		l.query,  // Query - nil means all events
		bookmark, // Bookmark - for resuming from a specific event

why return nil when err happens
thanks

system · December 28, 2018, 3:01am

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.