Jump to content

David Heffernan

Members
 View Profile  See their activity

  • Content Count

    3710

  • Joined

  • Last visited

  • Days Won

    185

Posts posted by David Heffernan

  1. Posted

    5 hours ago, Kas Ob. said:

    Why wrong?

    StatusOut is receiving in the original code does receive a const string and does forward it to Memo.Lines.Add which in turn send a message to the end of the message queue, mean while exiting the thread SendMessage lock/wait which in turn release the string. and second message that sent to Memo does have the original pointer from the thread stack( local variable there), Right ?

    Please correct me.

    The code in the original post, using SendMessage is fine. It passes a pointer to the first character of the string, and the call to SendMessage doesn't return until the message has been fully processed.

  2. Posted

    1 hour ago, Kas Ob. said:

    Hi, I  had to register here again to answer this and save you time.

     

    Andres example does in fact shows the problem, but assuming this will happen only with SendMessage is wrong, on contrary this happen more often with SendMessage, but not directly with the sent message itself.

     

    Let me explain:

    1_ First message sent by SendMessage a procedure in thread and that procedure does own the pointer to the that string and will free the memory assigned to it on exiting.

    2_ SendMessage is blocking that procedure till return, so far so good, and that pointer with its memory is OK and SAFE to be used in the receiving procedure.

    3_ The problem is here start, when the receiving procedure will handle the data, i mean you had data and you need to do something with it, like any normal output operation, so here is the thing, you might send the data to :

    A) device with handle like files,sockets.. etc

    B) will display the thing on control like Memo or ListBox ..

    with the second case most likely there will be another SendMessage sent to the control, and that receiving procedure will exit releasing the thread procedure -> releasing that pointer on stack and its memory, you might still see the data OK and that by chance as MemoryManager will not free it yet (per allocation it is there and accessible)

    With first one (A) the thing is even harder to find and debug because you might be using overlapped operation and those are the hardest and nastiest bugs to find !

     

    After all that, @David Schwartz may i ask if simply this solve this issue or not ? 

    
procedure StatusOut(const stat: string);
var
  AMsg:string;
begin
  AMsg := UniqueString(stat);
  LogWrite(AMsg, true, true);
  FrmMain.memStatus.Lines.Add(AMsg);
end;

     

     

    This is completely wrong. 

  3. Posted

    46 minutes ago, A.M. Hoornweg said:

    I wonder about sendmessage being executed in the context of the calling thread.  Is that documented somewhere?  The HWnd could be in an entirely different process, so there must be some kind of marshaling in between. 

    Yes it is documented in the remarks section of the SendMessage documentation.

     

    Parameters are marshaled for messages known by the system.  A good example is WM_COPY.  The data that you pass to that message is marshaled to the target process, because the system knows its structure.  For user defined messages, no marshaling of parameters is performed.

  4. Posted

    Do you understand that what you want is not supported, and any attempt to implement it will inevitably rely on implementation details that are subject to change?

     

    If you find the level of service that you are receiving from this site to be below expectations that I can only apologise and hope you have better luck elsewhere. 

  6. Posted · Edited by David Heffernan

    40 minutes ago, Anders Melander said:

    You are correct if the string resource is protected, but I'm not seeing any evidence of that.

    The string copy you're referring to happens in the message handler when the PChar is converted to a string, but before (or while) that happens the tread can have modified or free'd the source string. It's a race condition.

    The code in the original post uses SendMessage. Which is synchronous.

     

    What you say would be true if you used PostMessage. But that is not what is happening here. I guess that you were making that second point on the assumption that PostMessage had to be used, which I didn't pick up. 

     

    Debugging with tiny excerpts of code is kinda pointless. The asker can obtain a stack trace and take it from there. 

  7. Posted

    1 minute ago, Mike Torrettinni said:

    I did a quick look at some of the big component sources and you can actually find Pred() usage quite often. You can see some pas files use both options, Pred() and Count - 1. which I assume is due to different developers being involved.

    But this one is interesting: 

    
if Idx > pred(FItems.Count - 1) then

    and no comments. Since the same method already uses Pred(), I assume someone didn't like Count-1 and they changed it half way. 🙂

    Total obfuscation here. Just because it exists doesn't mean you should use it. 

  9. Posted

    Why do you call ProcessMessages? 

     

    And you say there are AVs but you don't have any information. The two addresses are critical, both the code address and the data address. And the call stack will be vital. Aren't you using madExcept or similar? Or if you have this under the debugger then you inspect call stack when the AV is raised. 

  10. Posted · Edited by David Heffernan

    6 hours ago, Anders Melander said:

    Okay. I'll try again then. There are multiple problems in the original code.

    Like I said, Application.ProcessMessages in itself isn't a problem. The problem is that when you use SendMessage then the WMStatusOut method will run in the context of the thread which means that Application.ProcessMessage will also be executed in the context of the thread and that will definitely end badly at some point. Replacing SendMessage with PostMessage will take care of that (and get rid of ProcessMessages while you're at it. It's the mark of an amateur).

     

    Once the above is fixed, the more serious problem will surface: You're passing a pointer to a string owned by the thread to the main thread. You haven't posted the thread code, but I'm betting that there's nothing in your code to prevent the thread from modifying, or even freeing, this string once StatusOut has been called. I'm sure one of the usual suspects will be happy to provide you with an alternative solution.

    I'm afraid that both the points stated here are factually incorrect. 

     

    SendMessage synchronously sends a message to a window. The code is executed by the thread that owns that window.

     

    Secondly, there is no reason why code in one thread should not operate on a string variable allocated in a different thread. That's not even what's going on anyway. It's just a pointer to null terminated character array. And the message handler receives that and makes a new string. No problem there. 

    • Like 1

  18. Posted

    58 minutes ago, Alexander Elagin said:

    Pred() and Succ() are simple iterators defined for enumerable simple data types. If only they could be overloaded for other data types (lists, collections, whatever... maybe even for yield support) their usefulness would be much higher.

    That doesn't make a lot of sense. What is the successor of a collection. Perhaps you mean the successor of a member of a collection. But that only makes sense in the context of the collection. 

×