1

I am getting a memory leak when I run try to read from Clipboard.

Sample code:

void SomeFunction()
{
   OpenClipboard(nullptr);
   HGLOBAL hglb = GetClipboardData(CF_TEXT);
   char* ch = static_cast<char*>(GlobalLock(hglb));

   CString rawClipboardData(ch);

   GlobalUnlock(hglb);
   CloseClipboard();
}

It is the middle row above which causes the memory leak according to Visual Studio. This row:

CString rawClipboardData(ch);

If I do not run it, there is no leak reported. But if I do run it I get the following debug output in visual studio output window:

Detected memory leaks!
Dumping objects ->
f:\dd\vctools\vc7libs\ship\atlmfc\src\strcore.cpp(158) : {75645} normal block at 0x00000000072C89A0, 52 bytes long.
Data: <`x              > 60 78 F7 D3 FE 07 00 00 0D 00 00 00 0D 00 00 00
Object dump complete.

Any ideas?

UPDATE: Added OpenClipboard(nullptr) in code above. Also in real code there are nullptr-checks. Just keeping it clean here to reduce amount of guard-clause code.

IInspectable
  • 46,945
  • 8
  • 85
  • 181
10100111001
  • 735
  • 15
  • 31
  • what is rawClipboardData? it doesn't appear to be anything standard. – xaxxon Oct 06 '16 at 09:02
  • Seems likely that the problem is coming from `CString` have you stepped into to it to see where any dynamic allocation is happening? – George Oct 06 '16 at 09:06
  • 1
    @xaxxon rawClipboardData is a CString variable that I simply create on the stack (sending in the "ch" char pointer into its constructor) to create a CString from a char* – 10100111001 Oct 06 '16 at 09:06
  • @10100111001 Yes but what is CString? Something will be either new'd or malloc'd inside, it doesn't matter that the CString obj itself is stack allocated. Unless of course the leak isn't actually coming from `CString rawClipboardData(ch);` – George Oct 06 '16 at 09:10
  • You should check all the nullptr results, also in your example code you dont seem to call `OpenClipboard` ? Otherwise this code looks correct – marcinj Oct 06 '16 at 09:13
  • @George Yes, I have now checked the strcore.cpp file at row 156 (as indicated by Visual Studio debug output for the memory leak and sure enough there is a malloc there). But the CString should handle its own memory. (This is CString which is part of MFC: https://msdn.microsoft.com/en-us/library/aa300688(v=vs.60).aspx) strcore.cpp file also has a Free method which frees the data it allocates using malloc, but for some reason it is not called. CString should clean up any held memory when my function above goes out of scope? – 10100111001 Oct 06 '16 at 09:16
  • @george Oh yes I missed that part when typing the question here. But the call is there (I shall edit my code sample in a moment). There are also nullptr checks. Just tried to keep the code to a minimum for cleanliness and also since I cannot copy-paste code from the machine I am running this on so I have to type by hand. – 10100111001 Oct 06 '16 at 09:18
  • @10100111001 I doubt that CString has such serious bugs, you may add breakpoint for the moment when this allocation is done with `_CrtSetBreakAlloc(75645)`, maybe its some other place. 75645 is the number from your log: `{75645}` – marcinj Oct 06 '16 at 09:21
  • @10100111001 Ok, you may have to just bite the bullet and debug your code. Just step through and figure out why free is not being called. Something that's a possibility though not a probability is the constructor failed after calling malloc, in which case the destructor wouldn't be called when the CString object is free'd off the stack. – George Oct 06 '16 at 09:21
  • @george I doubt it too, thanks for the tip I am trying to debug and it breaks at other parts in code but the 75645 is no longer the same allocation number every time I run the application it seems. Perhaps that is why it breaks at a completely unrelated spot. – 10100111001 Oct 06 '16 at 09:31
  • @George Yep, trying to debug, but Visual Studio refuses to enter the innards of CString when I press F11 on that line of code. :/ – 10100111001 Oct 06 '16 at 09:32
  • @xaxxon: CString is the string class supplied with MFC. (Actually, in some ways it's rather better than std::string.) – Martin Bonner supports Monica Oct 06 '16 at 09:58
  • @10100111001 The code you posted does much more than the title of this thread. Get rid of the clipboard code, `GlobalLock`, etc. and just initialize the CString with known data. If no leak is detected, then the issue is how you're handling the memory in the clipboard, or an issue with `GlobalLock` and `GlobalUnlock`, etc. – PaulMcKenzie Oct 06 '16 at 09:58
  • @10100111001: You to update the installation of Visual Studio to include the source for MFC – Martin Bonner supports Monica Oct 06 '16 at 10:00
  • 3
    We need a [mcve]. As it stands, your `CString` object is never used anywhere. The code you posted doesn't leak memory. If there **really** is a leak (which may not actually be the case), it is in the code we cannot see. – IInspectable Oct 06 '16 at 10:03
  • @MartinBonner Would try that by I do not currently have the rights to install stuff on this PC. – 10100111001 Oct 06 '16 at 10:14
  • @IInspectable The code does not need to use the CString object. The leak happens inside CString constructor when it runs some malloc call. The leak is happening at Microsoft file strcore.cpp, row 156. Visual Studio 2013. – 10100111001 Oct 06 '16 at 10:16
  • @PaulMcKenzie I included the GlobalLock and Unlock because I thought that perhaps this leak is somehow related to accessing Clipboard data. You know, perhaps someone would go "Aaah, yes when you run GlobalLock() and then create a CString object then [...] happens or [...] bug is exposed" or something like that, because I have never seen CString leak like that in other contexts in this way. Just happened now when trying to read data from clipboard. – 10100111001 Oct 06 '16 at 10:19
  • I can mention now that I have run Deleaker software and it also points to that very row where the CString object is created from the char pointer. But I still have no access to debug symbols to be able to dig beyond my own code and into Microsoft dlls. – 10100111001 Oct 06 '16 at 10:21
  • You need to set up your IDE to use [Microsoft's symbol server](https://msdn.microsoft.com/en-us/library/windows/desktop/ee416588.aspx#using_the_microsoft_symbol_server). On an unrelated note, have you verified, that your `CString`s d'tor runs? You can set up a conditional breakpoint to verify this. – IInspectable Oct 06 '16 at 10:25
  • I am stumped! I went to have lunch, I come back and now I can't reproduce the leak again. And suddenly Deleaker doesn't find the leak either....At this moment I am unsure what to think of this, but I shall keep an eye out for every change I do now in case it returns. @IInspectable thanks for the tip, but what condition should I set for the breakpoint for it to break on destruction? – 10100111001 Oct 06 '16 at 11:07
  • You set a breakpoint on construction, note the address of the internal data structure, and compare that address in the d'tor. That'll prevent all your other `CString` d'tors from triggering the breakpoint. Alternatively, use the address of the `CString` object itself, as that's more accessible than the internal data. – IInspectable Oct 06 '16 at 13:04

1 Answers1

-2

GlobalLock(hglb) should be a LPTSTR so I would assume that the leak is caused by the cast to char*. For Unicode platforms, TCHAR is defined as synonymous with the WCHAR type.

you should be able to do something like

CString rawClipboardData = GlobalLock(hglb);

If not then

CString rawClipboardData;
LPTSTR lptstr = GlobalLock(hglb);
rawClipboardData = lptstr;

will definitely work

jhbh
  • 317
  • 4
  • 11
  • 2
    That's just (poor) guessing. The return value of `GlobalLock` is an **input** to the `CString` c'tor. The c'tor makes a **copy** of the input data. Misinterpreting the input would lead to truncation of the stored value at worst. This **cannot** ever produce a resource leak. But that's all ignoring the fact, that [CF_TEXT](https://msdn.microsoft.com/en-us/library/windows/desktop/ff729168.aspx) is in fact returned as a `const char*` (vs. a `const wchar_t*` for `CF_UNICODETEXT`). – IInspectable Oct 06 '16 at 14:05