10

In Dephi, I create a thread, like this, which will send message to main form from time to time

Procedure TMyThread.SendLog(I: Integer);
Var
  Log: array[0..255] of Char;
Begin
  strcopy(@Log,PChar('Log: current stag is ' + IntToStr(I)));
   PostMessage(Form1.Handle,WM_UPDATEDATA,Integer(PChar(@Log)),0);
End;

procedure TMyThread.Execute;
var
  I: Integer;
begin
  for I := 0 to 1024 * 65536 do
  begin
    if (I mod 65536) == 0 then
    begin
      SendLog(I);
    End;
  End;
end;

where WM_UPDATEDATA is a custom message, defined below:

const
  WM_UPDATEDATA = WM_USER + 100;

And in main form, it will do as follows to update the list:

procedure TForm1.WMUpdateData(var msg : TMessage);
begin
  List1.Items.Add(PChar(msg.WParam));
end;

However, as the Log string sent to the main form is a local variable, which will be destroyed after calling SendLog. While TForm1.WMUpdateData process the message asynchronously, so it is possible that when it is invoked, the Log string has already been destroyed. How to solve this problem?

I think maybe I can allocate the string space in a global system space, and then pass it to the message, then after TForm1.WMUpdateData processes the message, it can destroy the string space in the global space. Is that a workable solution? How to implement this?

Thanks

4
  • 1
    Look here stackoverflow.com/questions/9932164/… Hopefully it helps
    – Arkady
    Sep 27, 2013 at 21:21
  • You need to declare your Log variable as a global variable .
    – S.MAHDI
    Sep 28, 2013 at 1:02
  • @S.MAHDI No! What if there are two queued messages? Sep 28, 2013 at 7:40
  • Consider putting these strings in a thread-safe list and posting a message back to the main thread to let it know you've added one. Sep 28, 2013 at 13:33

3 Answers 3

12

In addition to the fact that you are posting a local variable, the TWinControl.Handle property is not thread-safe, either. You should use the TApplication.Handle property instead, or use AllocateHWnd() to create your own window.

You do need to dynamically allocate the string on the heap, post that pointer to the main thread, and then free the memory when you are done using it.

For example:

procedure TForm1.FormCreate(Sender: TObject);
begin
  Application.OnMessage := AppMessage;
  // or use a TApplicationEvents component...
end;

procedure TForm1.FormDestroy(Sender: TObject);
begin
  Application.OnMessage := nil;
end;

procedure TForm1.AppMessage(var Msg: TMsg; var Handled: Boolean);
var
  S: PString;
begin
  if Msg.Message = WM_UPDATEDATA then
  begin
    S := PString(msg.LParam);
    try
      List1.Items.Add(S^);
    finally
      Dispose(S);
    end;
    Handled := True;
  end;
end;

procedure TMyThread.SendLog(I: Integer);
var
  Log: PString;
begin
  New(Log);
  Log^ := 'Log: current stag is ' + IntToStr(I);
  if not PostMessage(Application.Handle, WM_UPDATEDATA, 0, LPARAM(Log)) then
    Dispose(Log);
end;

Alternatively:

var
  hLogWnd: HWND = 0;

procedure TForm1.FormCreate(Sender: TObject);
begin
  hLogWnd := AllocateHWnd(LogWndProc);
end;

procedure TForm1.FormDestroy(Sender: TObject);
begin
  if hLogWnd <> 0 then
    DeallocateHWnd(hLogWnd);
end;

procedure TForm1.LogWndProc(var Message: TMessage);
var
  S: PString;
begin
  if Message.Msg = WM_UPDATEDATA then
  begin
    S := PString(msg.LParam);
    try
      List1.Items.Add(S^);
    finally
      Dispose(S);
    end;
  end else
    Message.Result := DefWindowProc(hLogWnd, Message.Msg, Message.WParam, Message.LParam);
end;

procedure TMyThread.SendLog(I: Integer);
var
  Log: PString;
begin
  New(Log);
  Log^ := 'Log: current stag is ' + IntToStr(I);
  if not PostMessage(hLogWnd, WM_UPDATEDATA, 0, LPARAM(Log)) then
    Dispose(Log);
end;
8
  • 3
    Allocating data and deallocating based on postmessage is not a thing that I would recommend. Your message may be never processed and then you create a memory leak that is hard to detect. Having not a single occurence of this pattern in the WINAPI may give an indication, that's not a good idea. For example, message queue sizes are limited. Or the target code may change and introduce the memory leak. Or the target window might handle WM_UPDATEDATA through the default window proc and so the memory never gets freed. And so on...
    – JensG
    Sep 28, 2013 at 7:49
  • @JensG Don't worry. This is the main form of the same app. The author of the app is in control of the handling of WM_UPDATEDATA. It's not difficult to verify that the memory is freed. And you are worried about a memory leak when your main thread's queue becomes full?! That's the least of your worries. My main thread message queue is full, but, OMG, I've leaked some memory!!!! Sep 28, 2013 at 10:33
  • @David Heffernan: How a solution that has several obvious drawbacks and despite the fact that there are better, more robust solutions is cheered that way is beyond my comprehension. And anyone who has ever tracked down strange memory leaks in some server app that is expected to run days and months without interruption but instead dies by sudden OOMs will agree to my comment. You just can't be careful enough about such things. You obviously haven't experienced that, otherwise you would not been talking that way. But of course it's great for you, so you can shine when it comes to bug searching.
    – JensG
    Sep 28, 2013 at 11:45
  • 1
    @JensG The fact that some people will be incapable of getting this right does not mean that it cannot be done. What Remy has demonstrated here is accurate and does not leak. Sep 28, 2013 at 12:00
  • 5
    There is no leak if the queue fills up. PostMessage() will fail, and then the original thread frees the memory it allocated since it could not be posted. Sep 28, 2013 at 15:37
12

If you have D2009 or later version, there is another way to post messages to your main form. TThread.Queue is an asynchronous call from a thread, where a method or procedure can be executed in the main thread.

The advantage here is that the frame to set up the message passing is less complex. Just pass your callback method when creating your thread. No handles and no explicit handling of string allocation/deallocation.

Type
  TMyCallback = procedure(const s : String) of object;

  TMyThread = class(TThread)
    private
      FCallback : TMyCallback;
      procedure Execute; override;
      procedure SendLog(I: Integer);
    public
      constructor Create(aCallback : TMyCallback);
  end;

constructor TMyThread.Create(aCallback: TMyCallback);
begin
  inherited Create(false);
  FCallback := aCallback;
end;

procedure TMyThread.SendLog(I: Integer);
begin
  if not Assigned(FCallback) then
    Exit;
  Self.Queue(  // Executed later in the main thread
    procedure
    begin
      FCallback( 'Log: current stag is ' + IntToStr(I));
    end
  );
end;

procedure TMyThread.Execute;
var
  I: Integer;
begin
  for I := 0 to 1024 * 65536 do
  begin
    if ((I mod 65536) = 0) then
    begin
      SendLog(I);
    End;
  End;
end;

procedure TMyForm.TheCallback(const msg : String);
begin
  // Show msg
end;

procedure TMyForm.StartBackgroundTask(Sender : TObject);
begin
  ... 
  FMyThread := TMyThread.Create(TheCallback);
  ...
end;
0

Use SendMessage().

PostMessage() will process your message asynchronously, it basically puts into the target message queue and Returns immediately. At the time where the handler code accesses the data sent in wparam/lparam, your caller has already freed the string.

In contrast, SendMessage() bypasses the message queue and calls the window proc directly (synchronously). At the time when SendMessage() returns, it is safe to free the string.

4
  • I may add, it's not the only solution and it may not be suitable in all cases resp. your specific case. In some cases, using Synchonize() may be another option to consider. It basically depends on how strict you want (or should) couple the both threads together. The losest approach would be an refcounted helper object used solely for data Exchange. The refcount makes sure that either thread is free to release it first without affecting the other. Of course the helper object needs a lock or some other mechanism to control concurrency. And there are even more possible solutions ...
    – JensG
    Sep 27, 2013 at 23:07
  • People choose PostMessage because they want asynchronous delivery. You ignore this and suggest synchronous instead. Which will have performance implications. Because now the worker threads have to wait for the main thread to be ready to dispatch the message. Sep 29, 2013 at 8:09
  • And that's why I added the comment that it might not be the only and/or best solution. Surely you have read this, haven't you? Your argument is only speculative, on top of that. The OP did nowhere write about his intentions why he chosed PostMessage.
    – JensG
    Sep 29, 2013 at 10:25
  • 1
    So bring that into your discussion in your answer. Explain the pros and cons of synch vs asynch. Sep 29, 2013 at 10:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.