cleaning up the memory in c++

[Doom9]

Having grown up with Java, ANSI C and C# I have recently had to write a client server DCOM program and I'm suspecting that somewhere along the lines I created a little memory leak, because with each new PC that connects, the memory usage of my window service goes up, and doesn't come down again when the client is no longer active (it goes down a bit, but not quite as much as it went up).
Having learned C I know that I have to free anything I allocate using malloc, but there's no such code here.
With objects usually allocating whatever memory they need, and cleaning up in the destructor, is there any need to say delete the pointer to the class (using the delete statement), or is the destructor automatically called anyway if the object goes out of scope? Does anything created with the keyword new have to be deleted again (with delete) after it is no longer used?

Also, I'm using CStrings and BSTRs quite a bit and I'm not quite sure if there's any cleaning up needed after them.
Here's some code I'm using to convert a BSTR to a CString.
_bstr_t tmp(Login, FALSE); //wrap the BSTR
CString cs(static_cast<const char *>(tmp)); //convert it

do I have to clean those up manually if I no longer use them (those are local variables)?

Also, to return I need BSTR pointers (pFirstname is a BSTR*)

*pFirstname = some_CString.AllocSysString();

could the AllocSysString potentially allocate memory that is never freed?

[Nic]

Well anything created with "new" should be "delete"d afterwards. But C++ objects created on the stack obviously do not need deleting and the deconstructor will clear up any memory used. (as long as the deconstructor supports it)

With:
_bstr_t tmp(Login, FALSE); //wrap the BSTR
CString cs(static_cast<const char *>(tmp)); //convert it

No clean up is necessary. CString will clean up cs when it goes out of scope (and hence its deconstructor called).

If doing:
while ( true )
unsigned short *pFirstname = some_CString.AllocSysString();

You'll get a memory leak. But normally the function receiving the BSTR will free the string when it is done using it. But you can free it yourself using:
SysFreeString(pFirstname);

[Doom9]

thanks for the explanation.. so it was like I assumed.. create with new, clean up with delete.

well.. the BSTR* I'm allocating is a remote parameter (with marshalling and everything), so I cannot clean it up because the content is only passed over the DCOM channel when the method returns.. so I assume that the marshaller will take care of the cleaning up.
Are there any other obvious things or is it just DCOMs way to blow up the service as users connect? There will be about 30 users total and I can't simulate this because the memory usage seems to depend only on the number of remote hosts (and I don't have 30 computers I can install the client on). So, assuming the seen amount of memory usage increase per user, I should be on the safe side, but I'm still a bit concerned. I think I'm not cleaning up the registry reading/writing class by myself now so I'm going to try to delete it when it's no longer needed.

[Tom|420]

Not a 100% sure from your post, but I will assume you are using Windows. If this is not Windows, it might apply either but possibly not.

Windows does some optimizing by actually allocating more memory than requested and not deallocating memory immediately.

That is, when your program is started, a bit more than the actual memory required/requested is physically allocated to your program, so that when (if) you request more memory some is already available. When you deallocate memory Windows might not free it up immediately, in case that you re-request it later.

Unless you are watching memory from a debugging program which has direct access to your code, the tool you are using reports what memory was allocated to your program by Windows and thus the numbers might not be suitable for debugging purpose.

From experience, when I programed network software I could see that memory allocated for new connections wasn't immediately and completely freed after disconnection. Since I am usually working with framework classes which I am pretty confident do cause memory leaks and that I had verified that all the classes created where destroyed, this is for sure an issue with how Windows handle memory, not with my software.

Also I have noticed that this is way more obvious with network applications than any other type of coding which would allocate and deallocate memory a lot, thus I suspect Windows does something very particular to memory used for network connections, probably because once a connection is closed it is almost certain than another connection will be created soon.

I did an experiment one by creating a certain number of connections from remote computers, closing the connections then reopenning the connections several times, and as long as the max number of active connections is never changed there doesn't seem to be more memory allocated.

If deconnecting and reconnecting the same client multiple times increases the memory allocated a memory leak is pretty obvious however.

In C++ allocating memory with 'new' never cause the memory to be released automatically. When the pointer goes out of scope, it is destroyed, but not it's value (which is just the address to the allocated memory). I don't believe this is a C++ weakness. Actually you could have assigned the address of that allocated memory to other pointers, and releasing the memory when only one of those pointers is deleted would ... well... I guess you see it :) I don't see how anything could be released automatically when dealing with pointers.


Hope this is still usefull even one month after your original post
Tom :)

[Doom9]

Incidentally the issue has come up again today.

The platform is indeed windows. Is there any good tool to check for memory leaks and if all declared variables are cleared again?

I'm especially concerned about stuff like

_bstr_t tmp(Login, FALSE); //wrap the BSTR

but also the BSTRs returned by COM calls:

// declaration
BSTR agent;
BSTR password;
// use
hr = server->GetLogin(login, &errorcode, &agent, &password);
if (FAILED(hr))
_com_issue_error(hr);
Error.Format("%u", errorcode);
Agent = W2A(agent);
Password = W2A(password);

errorcode, agent and password are filled on the server. After the above lines, I don't use errorcode, agent and password anymore. Should I add a couple of SysFreeString(stringname) afterwards? I know most COM templates automatically take care of cleaning up the memory of they go out of scope, but BSTR isn't one of them afaik.

Thus, if I pass a BSTR to the server, should it be cleaned up on the server after it is no longer used, and should the resulting BSTRs that are passed back be cleaned up on the client side (the code you see is on the clien side, the variable login is a BSTR that is passed to the server)?

[gabest]

I know most COM templates automatically take care of cleaning up the memory of they go out of scope, but BSTR isn't one of them afaik.It's not a class, only a typedef. BSTR is usually allocated by the called function and freed who receives it, but the documentation should state this clearly.

A little example with CComBSTR:#include <atlbase.h>

void funct(BSTR* s)
{
if(s) *s = SysAllocString(L"efwfewfEWF");
}

int main(int argc, char* argv[])
{
CComBSTR s;
funct(&s);
return 0;
}

[Doom9]

@gabest: but in your example, shouldn't you do a SysFreeString(&b) before returning?

Now what do you do with this?

client:

_bstr_t login = Username;
unsigned char errorcode = 0;
BSTR firstname;
BSTR lastname;
ULONG agent;
BSTR password;
ILoginPtr server = NULL;
...
...
hr = server->GetLogin([in]login, [out]&errorcode, [out]&firstname, [out]&lastname, [out]&agent, [out]&password);
...
output += " Vorname: ";
output += W2A(firstname);
output += " Nachname: ";
output += W2A(lastname);
output += " Agentnummer: ";
... // now I don't need my BSTRs anymore


server:

[quote]...
STDMETHODIMP CLogin::GetLogin(BSTR Login, BYTE* pError, BSTR* pFirstname, BSTR* pLastname, ULONG* pAgent, BSTR* pPassword)
{
...
_bstr_t tmp(Login, FALSE); //wrap the BSTR
CString cs(static_cast<const char *>(tmp)); //convert it
BOOL success = false;
...
...
*pError = 0;
*pFirstname = reg->ReadString("firstname", 0).AllocSysString();
*pLastname = reg->ReadString("lastname", 0).AllocSysString();
*pAgent = (ULONG)reg->ReadInt("agentnr", 0);
*pPassword = reg->ReadString("password", "").AllocSysString();
return S_OK;
}[/quote[

Now obviously I can't do a SysFreeString on pFirstname, pLastName, and pPassword on the server because only upon return S_OK will those values be sent back to the client. So, should I do a SysFreeString on firstname, lastname, and password on the client once I have processed those values and no longer need them? What about the login parameter? I found some examples where it is also freed using SysFreeString on the client. But is that enough? I still have a copy of it on the server, havn't I?

[gabest]

Originally posted by Doom9 @gabest: but in your example, shouldn't you do a SysFreeString(&b) before returning?No, because it is done by CComBSTR in its destructor when it goes out of scope (and unlike _bstr_t it has an BSTR* operator &() what makes it possible to pass it as &s)So, should I do a SysFreeString on firstname, lastname, and password on the client once I have processed those values and no longer need them? Yea, exaclty. What about the login parameter? I found some examples where it is also freed using SysFreeString on the client. But is that enough? I still have a copy of it on the server, havn't I?Called function allocates, caller frees. This doesn't mean the opposite too :) When the caller allocates something, it should also free it. However, the called function can make a copy to preserve it for later use.