KeSetEvent - event lifetimes.

Hiyaz,

Got something in my code which I suspect is a bug, but never seen a
problem with it … yet. It concerns the lifetime of events.

Thread 1:

NTSTATUS WT_CloseStreamWorkEntryAndWait(PSTREAM_WORK_POOL pPool, ULONG
StreamIdx)
{


//If no operations in progress, then can simply zero all flags and quit,
else
//need to set closing flag and wait on the event - the thread that wakes
us up
//should perform the final zeroing out of data.

status = KeWaitForSingleObject(&pEntry->StreamCloseEvent, Executive,
KernelMode, FALSE, NULL);
return status;
}

Seems OK so far?

How about: Thread 2:

case esIdle:
//If closing, then wake up main thread.
if (pEntry->Closing)
{
KdPrint3((“%s: WorkerThread: Detected entry closing flag, setting close
event.\n”, gDriverName));
//Okay, something is waiting for us to clearup this entry.
KeSetEvent(&pEntry->StreamCloseEvent, EVENT_INCREMENT, FALSE);
//Seems slightly paradoxical, but we can memset the data
//including the event, just after we’ve set it.
RtlZeroMemory(pEntry, sizeof(STREAM_WORK_ENTRY));
//Don’t need to keep working on behalf of this entry.
//This entry is definiely idle at the end of this!

My great concern with this code is that the thread that sets the event
then zeroes all the data out. I seem to recall there being an issue with
the Win32 “PulseEvent”, whereby threads that appeared to be waiting on
the event actually weren’t (Kernel mode APC delivery), and I’m concerned
that what I’m doing here might be risky.

Thanks for any help you might be able to provide.

MH.

This email and any attachments is confidential, may be legally privileged and is intended for the use of the addressee only. If you are not the intended recipient, please note that any use, disclosure, printing or copying of this email is strictly prohibited and may be unlawful. If received in error, please delete this email and any attachments and confirm this to the sender.

Thread 2 is broken code. There is nothing to prevent thread 2 from zeroing
out the pEntry object before thread 1 gets to its KeWait, and certainly
nothing to prevent thread 2 from zeroing out the event object while thread 1
is still using it. The comment is back-asswards. Thread one should zero out
the data structure when it wakes up as it is the one that has a defined
execution order with respect to thread 2, either that or you need a second
event to synchronize thread 2.

=====================
Mark Roddy
Windows .NET/XP/2000 Consulting
Hollis Technology Solutions 603-321-1032
www.hollistech.com


From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Martin C Harvey
Sent: Tuesday, February 08, 2005 5:46 AM
To: Windows System Software Devs Interest List
Subject: [ntdev] KeSetEvent - event lifetimes.

Hiyaz,

Got something in my code which I suspect is a bug, but never seen a problem
with it … yet. It concerns the lifetime of events.

Thread 1:

NTSTATUS WT_CloseStreamWorkEntryAndWait(PSTREAM_WORK_POOL pPool, ULONG
StreamIdx)
{


//If no operations in progress, then can simply zero all flags and quit,
else
//need to set closing flag and wait on the event - the thread that wakes us
up
//should perform the final zeroing out of data.

status = KeWaitForSingleObject(&pEntry->StreamCloseEvent, Executive,
KernelMode, FALSE, NULL);
return status;
}

Seems OK so far?

How about: Thread 2:

case esIdle:
//If closing, then wake up main thread.
if (pEntry->Closing)
{
KdPrint3((“%s: WorkerThread: Detected entry closing flag, setting close
event.\n”, gDriverName));
//Okay, something is waiting for us to clearup this entry.
KeSetEvent(&pEntry->StreamCloseEvent, EVENT_INCREMENT, FALSE);
//Seems slightly paradoxical, but we can memset the data
//including the event, just after we’ve set it.
RtlZeroMemory(pEntry, sizeof(STREAM_WORK_ENTRY));
//Don’t need to keep working on behalf of this entry.
//This entry is definiely idle at the end of this!

My great concern with this code is that the thread that sets the event then
zeroes all the data out. I seem to recall there being an issue with the
Win32 “PulseEvent”, whereby threads that appeared to be waiting on the event
actually weren’t (Kernel mode APC delivery), and I’m concerned that what I’m
doing here might be risky.

Thanks for any help you might be able to provide.

MH.

This email and any attachments is confidential, may be legally privileged
and is intended for the use of the addressee only. If you are not the
intended recipient, please note that any use, disclosure, printing or
copying of this email is strictly prohibited and may be unlawful. If
received in error, please delete this email and any attachments and confirm
this to the sender.

Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument: ‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

Thanks Mark - I’ve been rewriting the code today, and that’s precisely
what I do: there are some refcounts now in the function that waits
(thread1), so that it looks like this:

if (Close)
{
pEntry->IdleCloseFlags |= IDLE_CLOSE_FLAG_CLOSE;
pEntry->WaitingCloseCount++;
}
else
{
pEntry->IdleCloseFlags |= IDLE_CLOSE_FLAG_IDLE;
pEntry->WaitingIdleCount++;
}
KeReleaseSpinLock(&pPool->DataLock, Irql);
status = KeWaitForSingleObject(&pEntry->StreamIdleCloseEvent, Executive,
KernelMode, FALSE, NULL);
KeAcquireSpinLock(&pPool->DataLock, &Irql);
//Okay, we’re exiting, if last to exit from wait, then perform cleanup
if required.
if(Close)
pEntry->WaitingCloseCount–;
else
pEntry->WaitingIdleCount–;
if (WT_CheckCountsZero(pEntry))
{
//All threads waiting for idle or close have now been released from
event,
//if any of them specified the close, then do it now, otherwise, clear
flags.
//Of course, may need to wake up worker if pool completely closing.
if (pEntry->IdleCloseFlags & IDLE_CLOSE_FLAG_CLOSE)
WT_DestroyWorkEntry(pPool, pEntry, TRUE);
else
pEntry->IdleCloseFlags = 0;
}


From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Mark Roddy
Sent: 08 February 2005 12:13
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] KeSetEvent - event lifetimes.

Thread 2 is broken code. There is nothing to prevent thread 2 from
zeroing out the pEntry object before thread 1 gets to its KeWait, and
certainly nothing to prevent thread 2 from zeroing out the event object
while thread 1 is still using it. The comment is back-asswards. Thread
one should zero out the data structure when it wakes up as it is the one
that has a defined execution order with respect to thread 2, either that
or you need a second event to synchronize thread 2.

=====================
Mark Roddy
Windows .NET/XP/2000 Consulting
Hollis Technology Solutions 603-321-1032
www.hollistech.com


From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Martin C Harvey
Sent: Tuesday, February 08, 2005 5:46 AM
To: Windows System Software Devs Interest List
Subject: [ntdev] KeSetEvent - event lifetimes.

Hiyaz,

Got something in my code which I suspect is a bug, but never
seen a problem with it … yet. It concerns the lifetime of events.

Thread 1:

NTSTATUS WT_CloseStreamWorkEntryAndWait(PSTREAM_WORK_POOL pPool,
ULONG StreamIdx)
{


//If no operations in progress, then can simply zero all flags
and quit, else
//need to set closing flag and wait on the event - the thread
that wakes us up
//should perform the final zeroing out of data.

status = KeWaitForSingleObject(&pEntry->StreamCloseEvent,
Executive, KernelMode, FALSE, NULL);
return status;
}

Seems OK so far?

How about: Thread 2:

case esIdle:
//If closing, then wake up main thread.
if (pEntry->Closing)
{
KdPrint3((“%s: WorkerThread: Detected entry closing flag,
setting close event.\n”, gDriverName));
//Okay, something is waiting for us to clearup this entry.
KeSetEvent(&pEntry->StreamCloseEvent, EVENT_INCREMENT, FALSE);
//Seems slightly paradoxical, but we can memset the data
//including the event, just after we’ve set it.
RtlZeroMemory(pEntry, sizeof(STREAM_WORK_ENTRY));
//Don’t need to keep working on behalf of this entry.
//This entry is definiely idle at the end of this!

My great concern with this code is that the thread that sets the
event then zeroes all the data out. I seem to recall there being an
issue with the Win32 “PulseEvent”, whereby threads that appeared to be
waiting on the event actually weren’t (Kernel mode APC delivery), and
I’m concerned that what I’m doing here might be risky.

Thanks for any help you might be able to provide.

MH.

This email and any attachments is confidential, may be legally
privileged and is intended for the use of the addressee only. If you are
not the intended recipient, please note that any use, disclosure,
printing or copying of this email is strictly prohibited and may be
unlawful. If received in error, please delete this email and any
attachments and confirm this to the sender.

Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag
argument: ‘’
To unsubscribe send a blank email to
xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument:
‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

This email and any attachments is confidential, may be legally privileged and is intended for the use of the addressee only. If you are not the intended recipient, please note that any use, disclosure, printing or copying of this email is strictly prohibited and may be unlawful. If received in error, please delete this email and any attachments and confirm this to the sender.

Hello ,Martin

I think KeSetEvent satisfies the wait before return and Thread 1 does
not access the event any more, so you can safely zero it.

Tuesday, February 8, 2005, 12:46:12 PM, you wrote:

MCH> Got something in my code which I suspect is a bug, but
MCH> never seen a problem with it … yet. It concerns the lifetime
MCH> of events.
MCH>  
MCH> Thread 1:
MCH>  
MCH> NTSTATUS WT_CloseStreamWorkEntryAndWait(PSTREAM_WORK_POOL pPool, ULONG StreamIdx)
MCH> {

MCH> My great concern with this code is that the thread that
MCH> sets the event then zeroes all the data out. I seem to recall
MCH> there being an issue with the Win32 “PulseEvent”, whereby threads
MCH> that appeared to be waiting on the event actually weren’t (Kernel
MCH> mode APC delivery), and I’m concerned that what I’m doing here
MCH> might be risky.


Best regards,
Yura mailto:xxxxx@mail.zp.ua

What is the need of ever zeroing the KEVENT structure?

Maxim Shatskih, Windows DDK MVP
StorageCraft Corporation
xxxxx@storagecraft.com
http://www.storagecraft.com

----- Original Message -----
From: Martin C Harvey
To: Windows System Software Devs Interest List
Sent: Tuesday, February 08, 2005 1:46 PM
Subject: [ntdev] KeSetEvent - event lifetimes.

Hiyaz,

Got something in my code which I suspect is a bug, but never seen a problem with it … yet. It concerns the lifetime of events.

Thread 1:

NTSTATUS WT_CloseStreamWorkEntryAndWait(PSTREAM_WORK_POOL pPool, ULONG StreamIdx)
{


//If no operations in progress, then can simply zero all flags and quit, else
//need to set closing flag and wait on the event - the thread that wakes us up
//should perform the final zeroing out of data.

status = KeWaitForSingleObject(&pEntry->StreamCloseEvent, Executive, KernelMode, FALSE, NULL);
return status;
}

Seems OK so far?

How about: Thread 2:

case esIdle:
//If closing, then wake up main thread.
if (pEntry->Closing)
{
KdPrint3((“%s: WorkerThread: Detected entry closing flag, setting close event.\n”, gDriverName));
//Okay, something is waiting for us to clearup this entry.
KeSetEvent(&pEntry->StreamCloseEvent, EVENT_INCREMENT, FALSE);
//Seems slightly paradoxical, but we can memset the data
//including the event, just after we’ve set it.
RtlZeroMemory(pEntry, sizeof(STREAM_WORK_ENTRY));
//Don’t need to keep working on behalf of this entry.
//This entry is definiely idle at the end of this!

My great concern with this code is that the thread that sets the event then zeroes all the data out. I seem to recall there being an issue with the Win32 “PulseEvent”, whereby threads that appeared to be waiting on the event actually weren’t (Kernel mode APC delivery), and I’m concerned that what I’m doing here might be risky.

Thanks for any help you might be able to provide.

MH.

This email and any attachments is confidential, may be legally privileged and is intended for the use of the addressee only. If you are not the intended recipient, please note that any use, disclosure, printing or copying of this email is strictly prohibited and may be unlawful. If received in error, please delete this email and any attachments and confirm this to the sender.

Questions? First check the Kernel Driver FAQ at http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument: ‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

Just good housekeeping: the entry is not always used and has an
initialized flag to start with… once the entry is finished with, since
it’s not dynamically allocated (actually resides in device extension),
it need to be set to an “unused” value - it might be allocated later by
something else.

Anyways, you’ll be pleased to know that the close process has been
changed: several threads can now call a “WaitForClose” function, which
waits for all outstanding processing to complete, and then after the
wait, the threads re-enter a spinlock, decrement the ref count on the
entry, and the thread which finally decrements the count to 0 zeroes out
the structure, thus ensuring that when the event is zeroed, there will
not be any threads waiting on it.

FYI, a typical function using an entry looks like this:

NTSTATUS WT_RequestStreamWork(PSTREAM_WORK_POOL pPool, ULONG StreamIdx)
{
KIRQL Irql;
PSTREAM_WORK_ENTRY pEntry;

KdPrint4((“%s: WT_RequestStreamWork.\n”, gDriverName));

CHECK_STATUS(pPool, STATUS_INVALID_PARAMETER);
CHECK_STATUS(pPool->InitFlag == POOL_INIT_FLAG, STATUS_UNSUCCESSFUL);

KeAcquireSpinLock(&pPool->DataLock, &Irql);
pEntry = &pPool->WorkEntries[StreamIdx];
if (pEntry->InitFlag != ENTRY_INIT_FLAG)
{
KeReleaseSpinLock(&pPool->DataLock, Irql);
return STATUS_UNSUCCESSFUL;
}

So as you can see, that memset of everything to 0 does serve a useful
purpose.

MH.


From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Maxim S.
Shatskih
Sent: 09 February 2005 12:15
To: Windows System Software Devs Interest List
Subject: Re: [ntdev] KeSetEvent - event lifetimes.

What is the need of ever zeroing the KEVENT structure?

Maxim Shatskih, Windows DDK MVP
StorageCraft Corporation
xxxxx@storagecraft.com
http://www.storagecraft.com

----- Original Message -----
From: Martin C Harvey mailto:xxxxx
To: Windows System Software Devs Interest List
mailto:xxxxx
Sent: Tuesday, February 08, 2005 1:46 PM
Subject: [ntdev] KeSetEvent - event lifetimes.

Hiyaz,

Got something in my code which I suspect is a bug, but never
seen a problem with it … yet. It concerns the lifetime of events.

Thread 1:

NTSTATUS WT_CloseStreamWorkEntryAndWait(PSTREAM_WORK_POOL pPool,
ULONG StreamIdx)
{


//If no operations in progress, then can simply zero all flags
and quit, else
//need to set closing flag and wait on the event - the thread
that wakes us up
//should perform the final zeroing out of data.

status = KeWaitForSingleObject(&pEntry->StreamCloseEvent,
Executive, KernelMode, FALSE, NULL);
return status;
}

Seems OK so far?

How about: Thread 2:

case esIdle:
//If closing, then wake up main thread.
if (pEntry->Closing)
{
KdPrint3((“%s: WorkerThread: Detected entry closing flag,
setting close event.\n”, gDriverName));
//Okay, something is waiting for us to clearup this entry.
KeSetEvent(&pEntry->StreamCloseEvent, EVENT_INCREMENT, FALSE);
//Seems slightly paradoxical, but we can memset the data
//including the event, just after we’ve set it.
RtlZeroMemory(pEntry, sizeof(STREAM_WORK_ENTRY));
//Don’t need to keep working on behalf of this entry.
//This entry is definiely idle at the end of this!

My great concern with this code is that the thread that sets the
event then zeroes all the data out. I seem to recall there being an
issue with the Win32 “PulseEvent”, whereby threads that appeared to be
waiting on the event actually weren’t (Kernel mode APC delivery), and
I’m concerned that what I’m doing here might be risky.

Thanks for any help you might be able to provide.

MH.

This email and any attachments is confidential, may be legally
privileged and is intended for the use of the addressee only. If you are
not the intended recipient, please note that any use, disclosure,
printing or copying of this email is strictly prohibited and may be
unlawful. If received in error, please delete this email and any
attachments and confirm this to the sender.

Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag
argument: ‘’
To unsubscribe send a blank email to
xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument:
‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

This email and any attachments is confidential, may be legally privileged and is intended for the use of the addressee only. If you are not the intended recipient, please note that any use, disclosure, printing or copying of this email is strictly prohibited and may be unlawful. If received in error, please delete this email and any attachments and confirm this to the sender.</mailto:xxxxx></mailto:xxxxx>