blikblum/multilog

TMemoChannel is missing

Closed this issue · 9 comments

Logging channel for TMemo or TRichMemo should be implemented so I volunteer. I would use Synchronize for thread safety. I would probably base my implementation on TFileChannel.

Nice

Job done. Please review.

Example usage:

Application.CreateForm(TMyLogForm, MyLogForm);
Logger.Channels.Add(TMemoChannel.Create(MyLogForm.LogMemo); // form must first be created
Application.Run;

or

Application.CreateForm(TMyLogForm, MyLogForm);
Logger.Channels.Add(TMemoChannel.Create(MyLogForm.LogMemo, 'DD.MM.YYYY hh:nn:ss.zzz', 500 {lines limit if rotary buffer is used}, [fcoShowHeader, fcoShowTime, fcoRotaryBuffer]));
Application.Run;

I have left user possibility to change DateTime format string. If you like it then you can apply it elsewhere and close issue #3 "TFileChannel should not use fixed DateTime format".

Compilation gives this hint: "Conversion between ordinals and pointers is not portable". If you have a better way to pass a record to QueueAsyncCall() function then I am all ears.

All tests done on Windows XP SP2 with my SafeLogger helpers for thread safe logging. Tests used TFileChannel, TIPCChannel and TMemoChannel at the same time.

TMemoChannel can be a base for any other GUI channel in the future, like TRichMemo or TStringGrid. Even DB based ones are not far away.

I have tried to make license more clear and according to your license issue #8 reply. If you do not like this simplified text then change it whatever you want as long as it stays modified LGPL that allows static linking (as the rest of MultiLog lib). The only reason why I propose a text change is because it was not very clear. If I had trouble understanding what is exact lib license then probably someone else will have trouble too, and it would be a shame to loose a developer just because he wrongly thinks that lib can not be used in commercial applications. I also propose that license terms can be seen on front page of MultiLog repo. That is anyway what I first look when investigating a new lib.

TMemoChannel.zip

Any comment about my TMemoChannel implementation?

There are two issues:

  • if the memo is destroyed will cause an access violation if channel is used later. This can be implemented with a TComponent bridge or changing TChannel base call to TComponent. The later design is better but will be a breaking change
  • The constructor options is bit longer than desired. To make consistent with others it should accept only the target (Memo) and the options flags. Other options could be set as properties

I'll take some time to implement those.

I implemented MemoChannel based in your work. Made some modifications to be consistent with other components

In original LPR file I had something like this (try..except..finally are not relevant for this issue):

  Application.CreateForm(TCaster, Caster);
  Application.CreateForm(TBF, BF);
  Application.CreateForm(TSysTrayIcon, SysTrayIcon);
  Application.CreateForm(TVisualisationForm, VisualisationForm);
  //FormInitialized := true; // so threads know that form creation is finished // moved to InitFinished timer
  if FileExists(MEMO_START_CMD_FILENAME) then
    Logger.Channels.Add(TMemoChannel.Create(VisualisationForm.LogMemo, 'DD.MM.YYYY hh:nn:ss.zzz', 2000, [fcoShowHeader, fcoShowTime, fcoRotaryBuffer])); // switch on memo logging
  Application.Run;
  Logger.SendHeapInfo('Heap on program finish');

It worked without problems and without memory leaks with my initial TMemoChannel implementation. However after changing to latest Multilog which already has modified TMemoChannel, I got memory leaks because logger was trying to send log message string to Memo which was already destroyed. It didn't happen before with my implementation. I found a workaround, but I hope that you can take a look and maybe find a better solution. This is a workaround (whole LPR also attached):

  Application.CreateForm(TCaster, Caster);
  Application.CreateForm(TBF, BF);
  Application.CreateForm(TSysTrayIcon, SysTrayIcon);
  Application.CreateForm(TVisualisationForm, VisualisationForm);
  //FormInitialized := true; // so threads know that form creation is finished // moved to InitFinished timer
  if FileExists(MEMO_START_CMD_FILENAME) then
  begin
    MemoLogChannel := TMemoChannel.Create(VisualisationForm.LogMemo, [fcoShowHeader, fcoShowTime, fcoRotaryBuffer]);
    MemoLogChannel.TimeFormat := 'DD.MM.YYYY hh:nn:ss.zzz';
    MemoLogChannel.LogLinesLimit := 2000;
    Logger.Channels.Add(MemoLogChannel); // switch on memo logging
  end;
  Application.Run;

  if MemoLogChannel <> nil then // Assigned() might be better here
  begin
    Logger.Channels.Remove(MemoLogChannel);      // <<< WORKAROUND
    MemoLogChannel.Free;                         // <<< WORKAROUND
  end;
  Logger.SendHeapInfo('Heap on program finish'); // <<< PROBLEMS HERE IF WORKAROUND NOT USED

L1toOracle.zip

Do you mean memory leak or access violation ?

Sorry, I ment access violation.

It was one of the things I wrote down during testing new official threadsafe/memochannel version of MultiLog, and while adapting old code to new logic. However now it does not produce access violation even if I remove this whole block:

  if MemoLogChannel <> nil then // Assigned() might be better here
  begin
    Logger.Channels.Remove(MemoLogChannel);      // <<< WORKAROUND
    MemoLogChannel.Free;                         // <<< WORKAROUND
  end;

Strange. Sorry for the confusion...

Btw, new thread safe and memochannel version is now in production 24/7 without showing memory leaks and access violations so far. The old version was running for months without any problems. I will see what will say the test of time for the new version...

This line in MemoChannel:
TMemoChannelOption = (fcoShowHeader, fcoShowPrefix, fcoShowTime, fcoUnlimitedBuffer, fcoRotaryBuffer);
should be changed to:
TMemoChannelOption = (mcoShowHeader, mcoShowPrefix, mcoShowTime, mcoUnlimitedBuffer, mcoRotaryBuffer);
Otherwise there is a good chance that code complete in IDE editor will mix TFileChannelOption and TMemoChannelOption enumerators and bring confusion when typing. It happened to me today.

EDIT: It happened once more. Please fix it.