BorlandTalk.com Forum Index BorlandTalk.com
Borland discussion newsgroups
 
Archives   FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

IDE shuts down on second Build

 
Post new topic   Reply to topic    BorlandTalk.com Forum Index -> C++ Builder (VCL Components Development)
View previous topic :: View next topic  
Author Message
Royce Fessenden
Guest





PostPosted: Mon Mar 26, 2007 10:14 pm    Post subject: IDE shuts down on second Build Reply with quote



I have a component that will build and install ONCE with no errors or
warnings. But after it has been installed the IDE will shut down (screens
all disappear) when you try to build the package again. The component also
seems to work, though testing has not been extensive because we can't get
around the IDE shutdown problem.

What's wrong with this code?:
//---------------------------------------------------------------------------

#ifndef LoggerH
#define LoggerH
//---------------------------------------------------------------------------
#include <SysUtils.hpp>
#include <Classes.hpp>
//---------------------------------------------------------------------------

enum TDMPLogLevel
{
LL_DEBUG,
LL_INFORMATION,
LL_WARNING,
LL_EXCEPTION,
LL_CRITICAL
};

class PACKAGE TDMPLogger : public TComponent
{
private:
TDMPLogLevel FLogThreshold;
protected:
public:
__fastcall TDMPLogger(TComponent* Owner);

void __fastcall LogMessage(const AnsiString& className, const AnsiString&
functionName,
const AnsiString& detail, TDMPLogLevel level);
__published:
};
//---------------------------------------------------------------------------
#endif

//---------------------------------------------------------------------------

#include <basepch.h>

#pragma hdrstop

#include "Logger.h"
#include "SyncObjs.hpp"
#include <stdio.h>
//#include <io.h>
#pragma package(smart_init)

static TCriticalSection* g_LogQueueLock = new TCriticalSection();
static TEvent* g_LogQueueTrigger = new TEvent(NULL, true, false,
"LogMessageTrigger");

class TLogQueue
{
private:
TList* FData;

public:
TLogQueue()
{
FData = new TList();
}

~TLogQueue()
{
delete FData;
}

void AddMessage(const AnsiString& message)
{
try
{
g_LogQueueLock->Acquire();
AnsiString* pMessage = new AnsiString;
*pMessage = message;
FData->Add(pMessage);
}
__finally
{
g_LogQueueLock->Release();
g_LogQueueTrigger->SetEvent();
}
}

AnsiString GetMessage(bool& success)
{
success = false;
AnsiString message;
try
{
g_LogQueueLock->Acquire();
if (FData->Count > 0)
{
AnsiString* pMessage = (AnsiString*)FData->Extract(FData->First());
message = *pMessage;
delete pMessage;
success = true;
}
}
__finally
{
g_LogQueueLock->Release();
}

return message;
}
};

static TLogQueue* g_LogQueue = new TLogQueue;

class TLogRecorder : public TThread
{
public:
__fastcall TLogRecorder() : TThread(false)
{
}

void __fastcall Execute()
{
try
{
// Open/create file for appending text
AnsiString fileName = "c:\\Link\\TempLogFile.txt";
FILE* filePointer = fopen(fileName.c_str(), "a");


TWaitResult result = wrError;
while (result != wrAbandoned)
{
result = g_LogQueueTrigger->WaitFor(1000);
if (result == wrSignaled)
{
if (filePointer == NULL)
{
filePointer = fopen(fileName.c_str(), "a");
}

g_LogQueueTrigger->ResetEvent();
bool success = false;
AnsiString Message = g_LogQueue->GetMessage(success);

while (success)
{
fprintf(filePointer, "%s\n", Message);
OutputDebugString(Message.c_str());
Message = g_LogQueue->GetMessage(success);
}
Sleep(50);
}
else if (result == wrTimeout)
{
if (filePointer)
{
fclose(filePointer);
filePointer = NULL;
}
}
}

// TODO: make sure this gets closed properly
if (filePointer)
{
fclose(filePointer);
filePointer = NULL;
}
}
catch (const Exception& e)
{
// How do you log an error when the logger failed?
OutputDebugString(e.Message.c_str());
}
}
};

static TLogRecorder* g_LogRecorder = new TLogRecorder();
//---------------------------------------------------------------------------
// ValidCtrCheck is used to assure that the components created do not have
// any pure virtual functions.
//

static inline void ValidCtrCheck(TDMPLogger *)
{
new TDMPLogger(NULL);
}
//---------------------------------------------------------------------------
__fastcall TDMPLogger::TDMPLogger(TComponent* Owner)
: TComponent(Owner) , FLogThreshold(LL_DEBUG) // temporary
{
}
//---------------------------------------------------------------------------
namespace Logger
{
void __fastcall PACKAGE Register()
{
TComponentClass classes[1] = {__classid(TDMPLogger)};
RegisterComponents("DMP", classes, 0);
}
}
//---------------------------------------------------------------------------
void __fastcall TDMPLogger::LogMessage(const AnsiString& className, const
AnsiString& functionName,
const AnsiString& detail, TDMPLogLevel level)
{
if (level >= FLogThreshold)
{
AnsiString output = className + "::" + functionName + " | " + detail +
AnsiString(level);
g_LogQueue->AddMessage(output);
}
}
Back to top
Clayton Arends
Guest





PostPosted: Thu Mar 29, 2007 10:37 pm    Post subject: Re: IDE shuts down on second Build Reply with quote



Quote:
What's wrong with this code?:

The code you have shown has lots of memory leaks. If you require global
variables then you should either use std::auto_ptr or create your own class
to handle the memory management. ie:

#include <memory>

static std::auto_ptr<TCriticalSection> g_LogQueueLock(new
TCriticalSection);
static std::auto_ptr<TEvent> g_LogQueueTrigger(new TEvent(NULL, true,
false,
"LogMessageTrigger"));

However, in the case of the code you have shown g_LogQueueLock is not used
outside of the context of TLogQueue. This means that it should really be
added to the TLogQueue class. It appears that g_LogQueueTrigger is used to
signal from the global object TLogQueue to the global object TLogRecorder.
Since that is the case the event should also be added to TLogQueue.

If you need g_LogQueue to be global then it should be declared using
auto_ptr.

static std::auto_ptr<TLogQueue> g_LogQueue(new TLogQueue);

OR, since it is a C++ class that doesn't descend from a Pascal class you
don't have to use a pointer for it at all:

static TLogQueue g_LogQueue;

g_LogRecorder has an entirely different problem. Since it relies on the
existence of g_LogQueueTrigger its memory is tied to g_LogQueueTrigger's
existence. You can probably set FreeOnTerminate to true in the constructor
of TLogRecorder and be fine. Though, I've never liked to rely on the
deletion of an event to be a separate signal. Some redesign there might be
in order. For instance, if TLogQueue owned a TLogRecorder object (rather
than g_LogRecorder being global) it could terminate the thread in its
destructor and the TLogRecorder::Execute() loop could wait for that
condition rather than for the deletion of the event.

Other issues:

1. I could be wrong about this but I believe your fprintf() line needs to
use the c_str() result of your 'Message' and not 'Message' itself:

Quote:
fprintf(filePointer, "%s\n", Message);
fprintf(filePointer, "%s\n", Message.c_str());


2. Instead of allocating AnsiString pointers and placing them in TList use
TStringList for TLogQueue::FData instead.

3. When possible avoid the use of __finally. There are documented cases
where it doesn't work correctly in C++ code. Instead, create an RAII class
that performs the desired work. For example:

Create an RAII class that acquires and releases the critical section:

class RAII_CriticalSection
{
private:
TCriticalSection* cs;

public:
RAII_CriticalSection(TCriticalSection* Acs) : cs(Acs)
{ cs->Acquire(); }
~RAII_CriticalSection()
{ cs->Release(); }
};

And then convert the following code:

Quote:
void AddMessage(const AnsiString& message)
{
try
{
g_LogQueueLock->Acquire();
AnsiString* pMessage = new AnsiString;
*pMessage = message;
FData->Add(pMessage);
}
__finally
{
g_LogQueueLock->Release();
g_LogQueueTrigger->SetEvent();
}
}

to:

void AddMessage(const AnsiString& message)
{
RAII_CriticalSection auto_lock(g_LogQueueLock);
FData->Add(String()); // <-- assuming you changed FData to TStringList
g_LogQueueTrigger->SetEvent(); // <-- shouldn't have been in __finally
}

And convert the code:

Quote:
AnsiString GetMessage(bool& success)
{
success = false;
AnsiString message;
try
{
g_LogQueueLock->Acquire();
if (FData->Count > 0)
{
AnsiString* pMessage = (AnsiString*)FData->Extract(FData->First());
message = *pMessage;
delete pMessage;
success = true;
}
}
__finally
{
g_LogQueueLock->Release();
}

return message;
}

to:

AnsiString GetMessage(bool& success)
{
success = false;
AnsiString message;
RAII_CriticalSection auto_lock(g_LogQueueLock);
if (FData->Count > 0)
{
message = FData->Strings[0];
FData->Delete(0);

success = true;
}

return message;
}

Quote:
The component also seems to work, though testing has not been extensive
because we can't get around the IDE shutdown problem.

Once you get memory management under control try installing your component
again. If it still has a problem then gut your classes so that only the
fundamental functionality works. First step would be to comment out all of
your global variables, and the TLogQueue and TLogRecorder classes. Then in
TDMPLogger::LogMessage() comment out the call to AddMessage(). Once that
part is stable (which it should be) add functionality back one step at a
time.

However, here are all of the changes that I did to your code. I haven't
tested these so cannot vouch whether the code really compiles/works:

#include <memory>

class RAII_CriticalSection
{
private:
TCriticalSection* cs;

public:
RAII_CriticalSection(TCriticalSection* Acs) : cs(Acs)
{ cs->Acquire(); }
~RAII_CriticalSection()
{ cs->Release(); }
};

class TLogRecorder : public TThread
{
protected:
virtual void __fastcall Execute();

public:
TEvent* Trigger;

__fastcall TLogRecorder();
virtual __fastcall ~TLogRecorder();
};

class TLogQueue
{
private:
TStringList* FData;
TLogRecorder* Recorder;
TCriticalSection* Lock;

public:
TLogQueue();
~TLogQueue();

void AddMessage(const AnsiString& message);
AnsiString GetMessage(bool& success);
};

static std::auto_ptr<TLogQueue> g_LogQueue(new TLogQueue);

TLogQueue::TLogQueue()
{
Lock = new TCriticalSection;
FData = new TStringList;
Recorder = new TLogRecorder;
}

TLogQueue::~TLogQueue()
{
Record->Terminate();
delete FData;
delete Lock;
}

void TLogQueue::AddMessage(const AnsiString& message)
{
RAII_CriticalSection auto_lock(Lock);
FData->Add(String()); // <-- or use EmptyStr instead of String()
Recorder->Trigger->SetEvent();
}

AnsiString TLogQueue::GetMessage(bool& success)
{
success = false;
AnsiString message;
RAII_CriticalSection auto_lock(Lock);
if (FData->Count > 0)
{
message = FData->Strings[0];
FData->Delete(0);
success = true;
}

return message;
}


__fastcall TLogRecorder::TLogRecorder() : TThread(false)
{
Trigger = new TEvent(NULL, true, false, "LogMessageTrigger");
FreeOnTerminate = true;
}

__fastcall TLogRecorder::~TLogRecorder()
{
delete Trigger;
}

void __fastcall TLogRecorder::Execute()
{
try
{
// Open/create file for appending text
AnsiString fileName = "c:\\Link\\TempLogFile.txt";

FILE* filePointer = fopen(fileName.c_str(), "a");
// Or use the following since the fopen() is one of the first
// things to happen in the loop
// FILE* filePointer = NULL;

TWaitResult result = wrError;
while (!Terminated)
{
result = Trigger->WaitFor(1000);
if (result == wrSignaled)
{
if (filePointer == NULL)
{
filePointer = fopen(fileName.c_str(), "a");
}

Trigger->ResetEvent();
bool success = false;
AnsiString Message;
while (true)
{
Message = g_LogQueue->GetMessage(success);
if (!success)
break;
fprintf(filePointer, "%s\n", Message.c_str());
OutputDebugString(Message.c_str());
}
Sleep(50);
}
else if (result == wrTimeout)
{
if (filePointer)
{
fclose(filePointer);
filePointer = NULL;
}
}
}

// TODO: make sure this gets closed properly
if (filePointer)
{
fclose(filePointer);
filePointer = NULL;
}
}
catch (const Exception& e)
{
// How do you log an error when the logger failed?
OutputDebugString(e.Message.c_str());
}
}

//---------------------------------------------------------------------------
// ValidCtrCheck is used to assure that the components created do not have
// any pure virtual functions.
//

static inline void ValidCtrCheck(TDMPLogger *)
{
new TDMPLogger(NULL);
}
//---------------------------------------------------------------------------
__fastcall TDMPLogger::TDMPLogger(TComponent* Owner)
: TComponent(Owner) , FLogThreshold(LL_DEBUG) // temporary
{
}
//---------------------------------------------------------------------------
namespace Logger
{
void __fastcall PACKAGE Register()
{
TComponentClass classes[1] = {__classid(TDMPLogger)};
RegisterComponents("DMP", classes, 0);
}
}
//---------------------------------------------------------------------------
void __fastcall TDMPLogger::LogMessage(const AnsiString& className,
const AnsiString& functionName,
const AnsiString& detail, TDMPLogLevel level)
{
if (level = FLogThreshold)
{
AnsiString output = className + "::" + functionName + " | " +
detail + AnsiString(level);
g_LogQueue->AddMessage(output);
}
}

HTH,

- Clayton
Back to top
Display posts from previous:   
Post new topic   Reply to topic    BorlandTalk.com Forum Index -> C++ Builder (VCL Components Development) All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2006 phpBB Group
SEO toolkit © 2004-2006 webmedic.