Deleting memory which was allocated in DLL causes a crash

Question

3.50/5 (4 votes)

See more:

Dear all,

I have two modules, i.e., main.exe and A.dll. I have a function declared in a class, i.e., ClassA in the A.dll as follows:

C++

char* ClassA::ReadFile(const char *szFilePath)
{
    int length;
    char* buffer;
    if(!IsFileExist(szFilePath)) {
        return NULL;
    }
    std::ifstream is;
    is.open (szFilePath, std::ios::binary);

    // get length of file:
    is.seekg (0, std::ios::end);
    length = is.tellg();
    is.seekg (0, std::ios::beg);

    // allocate memory:
    buffer = new char [length +1 ];             //mark line1
    memset(buffer,0,length + 1);               //mark line2

    // read data as a block:
    is.read (buffer,length);
    buffer[length] = '\0';
    is.close();
    return buffer;
}

In the main.exe, I call the function as follows:

ClassA* pA = GetClassInstanceA();

char* pszBuffTemp = pA->ReadFile();
delete[] pszBuffTemp;  // crash here!

Here's the problem. When I run the main.exe, it comes to crash at the line "delete". But everything is ok after changing the "mark line1 and mark line2" above as follows:

C++

buffer = new char [length + 20];             //mark line1
memset(buffer,0,length + 20 );               //mark line2

That is to say, when I alloc the array 20 chars more than the real need, there's no problem. I don't know the reason why and thank you for all your attention!

////////////////////I tried it in the other sample as follows and the crash happened also///////////

////OutApi.h//////////

#ifndef _TEXT_OUTPUT_H_
#define _TEXT_OUTPUT_H_
#ifdef _WIN32
#	ifdef   TEXTDLL_EXPROTS
#		define  TEXTDLL_API   __declspec(dllexport)  
#	else
#		 define TEXTDLL_API   __declspec(dllimport)  
#	endif
#else
#	define TEXTDLL_API
#endif
#include <string>
class Iabc
{
public:
	Iabc() {}
	virtual ~Iabc() {}
	virtual char* GetText() = 0;
};
TEXTDLL_API Iabc* GetTestIns();
#endif

////////////////  CTest.H and CTest.CPP/////////

#ifndef _asdfasdfasf_H
#define _asdfasdfasf_H

#include "./OutApi.h"

class abc:  public Iabc
{
public:
    abc(){}
    ~abc(){}

    virtual char* GetText();
};

#endif

////////CTest.CPP/////////

#include "StdAfx.h"
#include "CTest.h"

char* abc::GetText()
{
    char* temp = "test code here ,if the problem still exists!";
    int len = sizeof(temp) + 20;
    char* pResult = new char[len];
    memset(pResult, 0 ,len);
    strcpy(pResult,  temp);
    return pResult;
}

Iabc* GetTestIns()
{
    static abc a;
    return &a;
}

//////////////the implematation of the exe ///////
#include "stdafx.h"
#include "OutApi.h"
#include <string>

#include <stdio.h>

#pragma comment(lib, "TextDll.lib")

int main(int argc, char* argv[])
{
    Iabc* abcObj = GetTestIns();
    char* pFromDll = abcObj->GetText();
    delete[] pFromDll;                    ///////crash here!

    int a;
    getchar(&a);
    return 0;
}

and the callstack information is as follows:

NTDLL! 7c92120e()
NTDLL! 7c98e576()
NTDLL! 7c9822e8()
KERNEL32! 7c85f9a7()
_CrtIsValidHeapPointer(const void * 0x00431800) line 1606
_free_dbg(void * 0x00431800, int 1) line 1011 + 9 bytes
operator delete(void * 0x00431800) line 49 + 16 bytes
main(int 1, char * * 0x00530e80) line 16 + 15 bytes
mainCRTStartup() line 206 + 25 bytes
KERNEL32! 7c817077()

Posted 10-Jun-10 23:26pm

xunbei100

Updated 11-Jun-10 4:15am

Deeksha Shenoy

v9

Add a Solution

Comments

Richard MacCutchan 11-Jun-10 7:11am

Have you tried stepping through this code with the debugger to see exactly what is happening? Are you sure that the pointer you are passing to delete has not been corrupted in any way?

xunbei100 11-Jun-10 7:17am

sure, I stepped through this code for several times and I checked the content of memory by the pointer value. and if I delete the array in the "readfile" function, there's no problem.

Niklas L 15-Jun-10 12:08pm

You do a lot of unnecessary memset calls. There is no point in clearing a buffer you directly after fill with characters, and where needed, a pBuffer[0] = 0 is doing just fine.

8 solutions

Solution 8

[Scenario]
Crash occurs when an application (EXE) tried to delete the memory allocated by another module (DLL).

[Solution]
In this case you have two heap managers in contention with each other; one for the EXE, and one for the DLL. Both of them are independent of each other, and both do not know anything about the other. The way around this is to use the DLL version of the MFC runtime libraries. This way there is only one heap manager for EXE and DLL, thus the issue can be solved. But this solution adds some restrictions to the DLL module in terms of who can use the DLL. (continue reading for details).

The other way of allocating or deallocating memory is to use the GlobalAlloc()/GlobalLock()/GlobalUnlock()/GlobalFree() functions using the GMEM_SHARE flag. These functions are actual Windows calls that get "real" memory from the OS. Since these functions all bypass the heap manager for the compiler, any application written in any language can call GlobalFree() on a handle that was allocated with GlobalAlloc(). This is the preferred way that DLL's allocate memory for external modules if it is up to the external module to free the memory.

As discussed in the first paragraph, if you solve this issue by correcting 'runtime library', a VB application cannot use the DLL as it does not know what 'new' and 'delete' is... So it is recommended to use 'Global...' functions for memory management (shared between modules).

Another solution to this issue is, provide 'ReleaseMemory' interface in shared DLL.

Choose the best one that suits your application...

Posted 17-Jun-12 23:29pm

Junaij

Solution 1

When you allocate your array, you forgot to take in consideration the string termination zero. You need to allocate your array for a size of length+1.
Remember that arrays in C++ are zero based, which means that when you allocate an array of 10 elements, the last element is at index 9 and not 10.

Posted 10-Jun-10 23:39pm

Cedric Moonen

Comments

xunbei100 11-Jun-10 5:42am

I am very sorry , My code post above was wrong ,the fact is that ,I did considered the termination zero of the array and my right code as fllow:

buffer = new char [length +1 ]; //mark line1
memset(buffer,0,length + 1); //mark line2

and the problem still exists!

Cedric Moonen 11-Jun-10 5:48am

Then it might be that your exe and your dll do not use the same heap. Make sure that they are both using the same version of the C runtime library (check that both the DLL and your exe have the same value for the "Runtime library" option in the project settings.

xunbei100 11-Jun-10 5:54am

I will have a litte simpler sample to test the problem and make it sure that if the problem still exists.
thank you for you help.

Solution 2

AFAIK
arrays in C are null terminated, which means that

xunbei100 wrote:
buffer[length] = '\0';

is obviously invalid assignment, because there's no such array element and it causes memory corruption.Perhaps you should use:

MIDL

buffer = new char [length+1 ];  
memset(buffer,0,length );
delete[] buffer;

Posted 10-Jun-10 23:47pm

Hristo-Bojilov

v2

Comments

xunbei100 11-Jun-10 5:49am

Thank you very much! I got a wrong code above, and I correct it fllow!

Solution 3

I am so sorry about my code post above, the right code of ClassA as fllow:

<pre lang="cs">char* ClassA::ReadFile(const char *szFilePath)
{
    int length;
    char* buffer;
    if(!IsFileExist(szFilePath)) {
        return NULL;
    }
    std::ifstream is;
    is.open (szFilePath, std::ios::binary);

    // get length of file:
    is.seekg (0, std::ios::end);
    length = is.tellg();
    is.seekg (0, std::ios::beg);

    // allocate memory:
    buffer = new char [length + 1]; //mark line1  here's length + 1
    memset(buffer,0,length + 1);    //mark line2

    // read data as a block:
    is.read (buffer,length);
    buffer[length] = '\0';
    is.close();
    return buffer;
}

and the problem exists the same

Posted 10-Jun-10 23:47pm

xunbei100

Comments

BillW33 19-Jun-12 14:34pm

You should not post a comment to a solution or an addition to your question as a "Solution". Either add a comment to a previous solution by pressing the "Have a Question or Comment?" button or update your question by using the "Improve question" button.

Solution 4

Here's a suggestion - don't use new and delete. You're making loads of work for yourself managing sizes and what nots, why not just use a vector? And you don't need to manually close files, that's one of the reasons they're objects.

std::vector<char> read_file( const std::string &file_name )
{
    std::vector<char> buffer;
    std::ifstream file_stream( file_name.c_str() );

    if( file_stream )
    {
        std::streamsize file_length = file_stream.seekg( 0, std::ios::end ).tellg();
        file_stream.seekg( 0, std::ios::beg );

        buffer.resize( file_length );
        file_stream.read( &buffer[ 0 ], file_length );
    }

    return buffer;
}

No tedious mucking about with arrays and fiddling about with memory allocators. And more importantly it's exception safe as well for when something goes wrong.

Posted 11-Jun-10 1:10am

Aescleal

Comments

xunbei100 11-Jun-10 7:22am

Thank you for you suggestion, it's a good idea. And I still wanna konw why my code went to crash.

Solution 5

You keep posting different pieces of code here so it's impossible to know what your actual program does; are you not able to do a simple copy and paste?

However the code below is wrong in that you have used sizeof(temp) as the length value (rather than strlen()) which will return 4 (the size of a char*), whereas the actual string is 44 characters in length, so even adding 20 you immediately corrupt your memory by the call to strcpy().

char* abc::GetText()
{
    char* temp = "test code here ,if the problem still exists!";
    int len = sizeof(temp) + 20;  // will return 24
    char* pResult = new char[len];
    memset(pResult, 0 ,len);
    strcpy(pResult,  temp); // oops, memory overwrite!!
    return pResult;
}

Posted 11-Jun-10 3:34am

Richard MacCutchan

Comments

xunbei100 11-Jun-10 10:22am

You are rigth, thks,I did make lots of mistakes ,sorry again.
I changed the "char* temp" to "char temp[]" ,and the result is the same.
May be Cedric is right,I will try to fix it by other ways.

Thank you again.

Richard MacCutchan 11-Jun-10 11:45am

"I will try to fix it by other ways." I explained above what you need to do; use the correct system call to find the length of your buffer and make sure you do not subsequently overwrite memory space that you do not own.

xunbei100 11-Jun-10 21:04pm

Follow your suggestion,I corrected my code. And I am quite sure about what my problem exactly be,
and the issue is still the crash caused by calling the function which return a char pointer pointing to a block memory allocated in the other dll.
And I had check the memory content by the pointer value,the memory is right,and I can get the right char array through the calling,
but it come to a crash when I delete the pointer to free the memory just following the calling.
PS: this problem only happened in the debug version, in the release version there's no problem ,
there's a Assert in the debug mode and it was omited under the release( "_CrtIsValidHeapPointer" ),
It is the problem that make me confused, if the memory heap is invalid ,how can I get the right char array?
Thank you very much for you help,I had a big lesson, ^_^

Solution 7

I had found the reason of the problem by the hint given by Cedirc,
only set the "use run-time library" to "Debug multithreaded dll" in setting of VC6.0 that you can new a block of memory in a dll and delete it in the other dll without meeting a debug assert.

Posted 18-Jun-10 5:05am

xunbei100

Updated 18-Jun-10 5:11am

v2

Comments

CPallini 18-Jun-10 11:39am

You may also link with the static runtime library, provided you move the cleanup code inside your DLL.

JackDingler 18-Jun-12 12:59pm

Please don't post additional questions and information, as solutions.

Use the 'Improve Question' link instead.

By posting in the solution panes, you're indicating that you've solved your problem and you're explaining how you solved it.

By reposting various of your question as answers, it's difficult to understand if you're still having a problem and what form your problem currently takes.

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Cedric Moonen · Accepted Answer · 2010-06-11T03:50:00

Solution 6

Did you make sure that bot hthe exe and the dll are using the same heap space ? This is extremly important and if they don't, that's certainly the source of your problem.

Posted 11-Jun-10 3:50am

Cedric Moonen