Dealing with HANDLEs

[Ben Menashe]

Hi,
So I wrote a class to describe a 'process'. One of the member variables is
a HANDLE. Some of the functions in the class (like GetProcessName) require
the handle to be open. So I open it once if not open, then other
operations can simply use the open handle. On the destructor I have this:

if(m_hProcess != NULL && m_hProcess != INVALID_HANDLE_VALUE)
{
CloseHandle(m_hProcess);
}

It works fine.... until I copy the object. If I make copies of it, some of
the destructors cause an invalid handle exception. So my question is, do I
need to copy the handle in some special way ? I tried to overload the
assignment operator and use DuplicateHandle to copy that value of the class,
but it didn't help. Also, why is the CloseHandle being fired if the handle
is truly invalid.... maybe there's a better way to check ?

Thanks !!

Ben

[Olof Lagerkvist]

Ben Menashe wrote:

> Hi,
> So I wrote a class to describe a 'process'. One of the member variables is
> a HANDLE. Some of the functions in the class (like GetProcessName) require
> the handle to be open. So I open it once if not open, then other
> operations can simply use the open handle. On the destructor I have this:
>
> if(m_hProcess != NULL && m_hProcess != INVALID_HANDLE_VALUE)
> {
> CloseHandle(m_hProcess);
> }
>
> It works fine.... until I copy the object. If I make copies of it, some of
> the destructors cause an invalid handle exception. So my question is, do I
> need to copy the handle in some special way ? I tried to overload the
> assignment operator and use DuplicateHandle to copy that value of the class,
> but it didn't help.

This should have worked, it has worked for me when I have encapsulated
kernel objects within C++ classes. But I have always overloaded not only
the assignment operator but also the copy constructor.

--
Olof Lagerkvist
ICQ: 724451
Web: http://here.is/olof

[Jake Forson]

As Olof already pointed out, it does work using "DuplicateHandle()". Make
sure your copy constructor and copy assignment operator are both invoking
this. Your copy assignment operator should also be closing the existing
handle before reassigning it to the (newly) duplicated handle. Your
destructor should also set the encapsulated handle back to NULL after
closing it (since your destructor tests this before closing it). You're
almost certainly not following one of these rules which (probably) accounts
for your problem (the same rules that apply for any other resource that
needs to be duplicated via a copy constructor and copy assignmet operator in
C++). You should also consider writing a separate wrapper class around a
HANDLE if time permits. I have my own that I've been using problem-free for
years but unfortunately I can't release it for legal reasons (sorry). If
you do create one make sure to create a virtual "Close()" function however.
It will invoke "CloseHandle()" as the default behaviour but you can then
override it for registry key handles and other objects that have their own
unique close function ("RegCloseKey()" for instance - don't know why MSFT
isn't consistent).

[Ben Menashe]

So I never tried both at the same time, I had either the copy construcor or
the assignment operator. However, you say i need to close the HANDLE of
the existing object when copying ?? is there a way to avoid this ? I don't
want it closed, since i want all the operations that need the handle to keep
working on both objects.

Any ideas ? do i just need to open a new handle using OpenProcess on a
copy/assignment operation ? I was trying to avoid that just to reduce the
load. (as this system has to deal with potentially a large number of these
operations). I wish I could just copy the handle in a way that makes the
two copies independent of each other so if one is closed, it doesn't affect
the other.

"Jake Forson" <no_spam@_nospam.com> wrote in message
news:ezSJ73MF...@TK2MSFTNGP06.phx.gbl...

[Jake Forson]

> So I never tried both at the same time, I had either the copy construcor
> or the assignment operator. However, you say i need to close the HANDLE
> of the existing object when copying ?? is there a way to avoid this ? I
> don't want it closed, since i want all the operations that need the handle
> to keep working on both objects.
>
> Any ideas ? do i just need to open a new handle using OpenProcess on a
> copy/assignment operation ? I was trying to avoid that just to reduce
> the load. (as this system has to deal with potentially a large number of
> these operations). I wish I could just copy the handle in a way that
> makes the two copies independent of each other so if one is closed, it
> doesn't affect the other.

Can I use your return email address? I'll send you an early draft of my
class with certain things stripped out. It's not the final production
version which is more extensive (with improvements, more functions, full
documentation, etc.) but it's not bad for an early copy. The important thing
is that it works as I recall. You'll have to remove or replace the exception
handling to make it compile however since it relies on exceptions in the
larger library which the class is part of. Other than that it has no
dependencies besides the WinAPI itself. I'll send you further instructions
when you confirm your email address.

[Le Chaud Lapin]

Ben Menashe wrote:
[Snip] I wish I could just copy the handle in a way that makes the

> two copies independent of each other so if one is closed, it doesn't affect
> the other.

You can. DuplicateHandle works 100%. As have others, I have been using
it for years with no problem:

First, I think Jake said something he did not mean. If you do a copy
construction or an assignment, you should _not_ close the handle in the
source object before doing the copy or assignment. Doing that might
cause the the object can go away completely, which will deny you the
opportunity to reference it.

Copy Constructor:

Waitable::Waitable (const Waitable &that)
{
DuplicateHandle(
GetCurrentProcess(),
(HANDLE) that.handle,
GetCurrentProcess(),
(HANDLE *) &this->handle,
NULL,
FALSE,
DUPLICATE_SAME_ACCESS);
}

Assignment Constructor:

When you do an assignment, do not forget to make sure the assignment is
not an A=A; situation:

Waitable & Waitable::operator = (const Waitable &that)
{
if (this != &that)
{
CloseHandle((HANDLE) this->handle);
DuplicateHandle(
GetCurrentProcess(),
(HANDLE) that.handle,
GetCurrentProcess(),
(HANDLE *) &this->handle,
NULL,
FALSE,
DUPLICATE_SAME_ACCESS);
}
return *this;
}

-Le Chaud Lapin-

[Arnaud Debaene]

"Ben Menashe" <benm...@gmail.com> a écrit dans le message de news:
zRHbh.38$r71...@newsfe02.lga...

Think more about it : Are you sure you want your "Process" class to be
copiable? A process is a unique entity, with it's own identity (it's
ProcessID), which can't be duplicated : the closest thing to copying a
process is forking it, which is quite unnnatural on Windows and is NOT a
mere copy.
It should be the same thing with your class : it shouldn't be copiable In
that case, making it's copy constructor private and undefined should do the
trick.

Now, if your class is more a "ProcessInformation" than a "Process", and you
really want to be able to copy it, DuplicateHandle is the way to go (in both
the copy constructor and the asignement operator). Show us your code if you
need help about it.

Arnaud
MVP - VC

>
> Thanks !!
>
> Ben
>

[Pavel Lebedinsky [MSFT]]

"Arnaud Debaene" wrote:

> Think more about it : Are you sure you want your "Process" class to be
> copiable? A process is a unique entity, with it's own identity (it's
> ProcessID), which can't be duplicated : the closest thing to copying a
> process is forking it, which is quite unnnatural on Windows and is NOT a
> mere copy.
> It should be the same thing with your class : it shouldn't be copiable In
> that case, making it's copy constructor private and undefined should do
> the trick.
>
> Now, if your class is more a "ProcessInformation" than a "Process", and
> you really want to be able to copy it, DuplicateHandle is the way to go
> (in both the copy constructor and the asignement operator).

Duplicated handles are not necessarily interchangeable. Each handle
has its own flags that can be set with SetHandleInformation for
example. The actual numeric value of the handle is also often used
for things like tracing/debugging, or even passing it as a command
line argument to child processes (for inheritable handles).

So I don't think using DuplicateHandle in a copy constructor or
operator= is a good idea. I would just make this class non-copyable.

--
This posting is provided "AS IS" with no warranties, and confers no
rights.

[adeb...@club-internet.fr]

On 1 déc, 09:28, "Pavel Lebedinsky [MSFT]" <p...@online.microsoft.com>
wrote:

> "Arnaud Debaene" wrote:
> > Think more about it : Are you sure you want your "Process" class to be
> > copiable? A process is a unique entity, with it's own identity (it's
> > ProcessID), which can't be duplicated : the closest thing to copying a
> > process is forking it, which is quite unnnatural on Windows and is NOT a
> > mere copy.
> > It should be the same thing with your class : it shouldn't be copiable In
> > that case, making it's copy constructor private and undefined should do
> > the trick.
>
> > Now, if your class is more a "ProcessInformation" than a "Process", and
> > you really want to be able to copy it, DuplicateHandle is the way to go

> > (in both the copy constructor and the asignement operator).Duplicated handles are not necessarily interchangeable. Each handle

> has its own flags that can be set with SetHandleInformation for
> example. The actual numeric value of the handle is also often used
> for things like tracing/debugging, or even passing it as a command
> line argument to child processes (for inheritable handles).
>
> So I don't think using DuplicateHandle in a copy constructor or
> operator= is a good idea. I would just make this class non-copyable.

I agree. It is coherent with the fact, that, if the class is really a
"ProcessInformation" class, there is no need for it to hold a handle to
the process as a data member (the handle should be opened just when
needed, in order to call APIs, then closed immediatly).

Arnaud
MVP - VC

[Jake Forson]

> I agree. It is coherent with the fact, that, if the class is really a
> "ProcessInformation" class, there is no need for it to hold a handle to
> the process as a data member (the handle should be opened just when
> needed, in order to call APIs, then closed immediatly).

I disagree with both of you. There is nothing intrinsically wrong or harmful
with copying a handle. In practice some minimal caution needs to be
exercised but this is easy enough. As you stated, it should also be opened
and closed at the earliest opportunity so the underlying object can be
removed as soon as possible (depending on who else holds a handle to the
object at the time). This applies to all resources however. In practice,
copying a handle is actually a rare occurence. Where it really comes into
play in the real world (typically) is when you've encapsulated a handle in a
class and then need to copy that object (the subject of this post). Maybe
you have an STL collection of those objects and by definition those objects
must have proper copy semantics (although typically you'll have collections
of pointers to support polymorphic behaviour). Caching of the handle may
also be necessary for performance or other reasons. A blanket statement that
the encapsulated handle shouldn't be duplicated (by privatizing the copy
constructor and copy assignment operator) means that you can't copy your own
object. This is completely unreasonable IMO and normally unecessary.
Programmers should exercise good judgment and caution at all times (a rarity
in real life) but I've been using my own HANDLE wrapper for many years and
have never had any issue (and I've been in the coding trenches longer than
most).

[Jake Forson]

> [Snip] I wish I could just copy the handle in a way that makes the
>> two copies independent of each other so if one is closed, it doesn't
>> affect
>> the other.
>
> You can. DuplicateHandle works 100%. As have others, I have been using
> it for years with no problem:
>
> First, I think Jake said something he did not mean. If you do a copy
> construction or an assignment, you should _not_ close the handle in the
> source object before doing the copy or assignment. Doing that might
> cause the the object can go away completely, which will deny you the
> opportunity to reference it.

I meant that the usual C++ copy rules surrounding resources apply. If you
copy A to B using a copy assignment operator for instance, B should first
copy A's resources (which must precede the following step if an error can
occur), release its own resources, and then take ownership of the copied
resources. You don't release A's resources at all. You'll normally have a
"const" reference to it in fact.

[Le Chaud Lapin]

Jake Forson wrote:
> I disagree with both of you. There is nothing intrinsically wrong or harmful
> with copying a handle. In practice some minimal caution needs to be
> exercised but this is easy enough. As you stated, it should also be opened
> and closed at the earliest opportunity so the underlying object can be
> removed as soon as possible (depending on who else holds a handle to the
> object at the time). This applies to all resources however. In practice,
> copying a handle is actually a rare occurence. Where it really comes into
> play in the real world (typically) is when you've encapsulated a handle in a
> class and then need to copy that object (the subject of this post). Maybe
> you have an STL collection of those objects and by definition those objects

> must have proper copy semantics [snip].

I entirely agree with Jake.

Encapsulating a HANDLE inside a C++ is highly regular. In fact, it is
one of the few places in Windows where the application of C++ to "clean
things up" doesn't have any adverse side effects. Note that this only
applies to objects with true reference counting, not something like a
WINDOW HANDLE where the behavior is quite contrary to KOBJECTS.

There was a post not long ago in [comp.lang.c++.moderated] by a
programmer who had the exact same issue as the OP. I tried to telling
him that the solution was DuplicateHandle, but he didn't listen, and
because the other responders had not enjoyed the pleasure of C++
encapsulation, they had not enjoyed its benefits, so there were all
kinds of answers given that were simply suboptimal. If the OP had
simply tried DuplicateHandle (and set NULL's in the right places), his
troubles would have been finished.

This OP should try it, then come back three months later and thank us.
:)

The object DACLS are a different matter. Since the copying is always
done inside the same process and what is wanted in most cases is just a
second object whose lifetime would be governed by the regularity of the
constructor/destructor semantics of C++, this is not a problem. In my
model, I simply let each object have the same security attributes, as I
never need anything more, but it is conceivable to define the
constructor of the C++ class so that attributes can be defined
on-the-fly.

Under this model, one *never* has to worry about over-referencing or
under-referencing a kernel mode object (assuming C++ mechanisms are
being employed correctly).

-Le Chaud Lapin-

[adeb...@club-internet.fr]

On 1 déc, 16:45, "Le Chaud Lapin" <jaibudu...@gmail.com> wrote:
> Jake Forson wrote:
> > I disagree with both of you. There is nothing intrinsically wrong or harmful
> > with copying a handle.

I agree, the problem is not a technical one : It is perfectly ok to use
DuplicateHandle as long as you play along the rules, but that's not the
point...
The problem I noticed is about object model : It's just rather
unnatural that a "Process" class is copyable. If there were such a
class, I would expect than, when an instance is duplicated, a new
process is launched (forked) on the system, and I doublt that is what
the OP wants
"Process" is just an identity object, who shouldn't be copyable for
semantic reasons ; as Jake as said, if you need to put them in an STL
collection, use (smart)pointers.

Arnaud
MVP - VC

[Ben Menashe]

First , WOW - thanks for all the replies... it's great to have this forum to
share with others past experiences.

I still had issues even when using DuplicateHandle (it ran... but in debug,
you could see all the exceptions in the trace).

So I ended up changing the design so it's not copyable. So this Process
class object lives as long as the process I detected does. It leads me to
another quick question, maybe someone can comment. So I need to detect
processes on start and exit and record the usage time. On the process open
event I create this Process class object, and open the HANDLE and add the
object to a <list>. To determine when processes shut down I scan this list
periodically and use GetProcessTimes on that handle to detemine if and when
the process shut down. This seems to work great. So even though the
process is dead already I can still get this info from the HANDLE -- that's
great !! My question is, does the handle stay valid forever (until I call
CloseHandle on it ?) -- or is there a risk of it becoming invalid if the
process has been dead for a while ?

Thanks so much everyone !!
Ben

<adeb...@club-internet.fr> wrote in message
news:1164991085.7...@79g2000cws.googlegroups.com...

[Le Chaud Lapin]

adeb...@club-internet.fr wrote:
> I agree, the problem is not a technical one : It is perfectly ok to use
> DuplicateHandle as long as you play along the rules, but that's not the
> point...
> The problem I noticed is about object model : It's just rather
> unnatural that a "Process" class is copyable. If there were such a
> class, I would expect than, when an instance is duplicated, a new
> process is launched (forked) on the system, and I doublt that is what
> the OP wants
> "Process" is just an identity object, who shouldn't be copyable for
> semantic reasons ; as Jake as said, if you need to put them in an STL
> collection, use (smart)pointers.

Salut,

I have to agree with you on the principle of natural conception. I did
look back and see that he is working with processes.

The technique Jake and I were proposing works for other kernel-mode
objects like mutexes, semaphore, etc. As for processes (and thread),
you're correct - there are some semantic issues that cannot be
overlooked. In particular, once you start controlling the underlying
mechanism (pausing, etc.) then there are other issues.

But...if you are working with other synchronization objects like
events, etc... then the C++ copy/assignment framework using
DuplicateHandle works well.

-Le Chaud Lapin-

[Le Chaud Lapin]

Ben Menashe wrote:
[snip] My question is, does the handle stay valid forever (until I call

> CloseHandle on it ?) -- or is there a risk of it becoming invalid if the
> process has been dead for a while ?
>
> Thanks so much everyone !!
> Ben

There is only one process object in the kernel (for sake of this
discussion) and N handles to that one object. As long as you have not
invoked CloseHandle on all handles, the object will linger. This is
why it is possible for a process to have "terminated" but still allow
its attributes to be determined - there is at least one handle open to
the objec that prevents the kernel from getting rid of the process. In
such cases, make no mistake: the kernel very much regards the lingering
blob of state as a process. As you can guess, this model is highly
regular. There is an analgous mechanism for threads. In fact, this
mechanism is appropriate for almost all handle-based entitities in
Windows, except for things like threads.

See a previous post:
http://groups.google.com/group/microsoft.public.win32.programmer.kernel/browse_frm/thread/5a65858a89d5f5d8/8f0451d1e53f733a?lnk=st&q=Le+Chaud+Lapin+real+process&rnum=2&hl=en#8f0451d1e53f733a

-Le Chaud Lapin-

[Ben Menashe]

Ok, so last tips i'm asking for.... suddenly the release ver of my code
started leaking handles, and i'm not sure what caused it. Everytime i've
seen this before, I was able to reproduce in debug mode and slowly find the
root. But this time, in debug it works fine. And this is not a small
leak... every 10 sec. it leaks about 70 handles (the # of running procs)
since it does a scan on all those processes using that class.

But I'm opening the handle only once, and closing on destructor now.... AND,
it works in debug. So I'm confused. Is there any good techniques for
debugging this ?

Thanks !!

"Le Chaud Lapin" <jaibu...@gmail.com> wrote in message
news:1165002981.3...@l12g2000cwl.googlegroups.com...

[Ben Menashe]

I found the issue with the leaks. Kinda strange... maybe just since I
don't understand it:

On every new process event this program creates a new Process object and
passes it for processing. First thing before processing I pass a pointer of
this object to a function that looks at the <list> of processes the service
looked at already and if the PID is found there it deletes the passed
pointer and assigns the pointer from the <list> to it.

The declaration was:

void GetPointerOfProcessFromList( CProcess* pProcess)

This worked in debug. But in release it didn't seem to return the pointer
correctly, and that's why it called OpenProcess on the object again. When
I changed the function to return the pointer it worked:

CProcess* GetPointerOfProcessFromList( CProcess* pProcess)

Strange, I thought if I pass a pointer, it's like by ref, I can change its
address and it changes the passed object, not a copy of it. Also, it worked
in debug. I know it's getting out of the scope of this thread... but just
wanted to mention what the issue was.

Thank you all for the support!

"Ben Menashe" <benm...@gmail.com> wrote in message
news:cO1ch.38$lg3...@newsfe02.lga...

[m]

<snip>

> void GetPointerOfProcessFromList( CProcess* pProcess)
>
> This worked in debug. But in release it didn't seem to return the
> pointer correctly, and that's why it called OpenProcess on the object
> again. When I changed the function to return the pointer it worked:
>
> CProcess* GetPointerOfProcessFromList( CProcess* pProcess)
>
> Strange, I thought if I pass a pointer, it's like by ref, I can change its
> address and it changes the passed object, not a copy of it. Also, it
> worked in debug. I know it's getting out of the scope of this thread...
> but just wanted to mention what the issue was.
>

This can't work ever; and if it does, it it by coincidence. This is basic
language info but FYI:

When you pass a pointer (non const), you can modify whatever it pointed to
by that pointer but the value of the pointer is stored on the stack, just
like any other parameter, and so cannot be modified. if you want to modify
the value of a pointer (ie make it point to somthing else), then you must
pass a pointer to the pointer.

for example:
void func1(CProcess** ppProcess)
{
CProcess* pProcess = *ppProcess; // get the input value
// do stuff and assign to pProcess
// now set the output
*ppProcess = pProcess;
}

// sample call
func1(&pProcess);