Dalija Prasnikar

I don't use Python4Delphi so I cannot accurately tell what can and what cannot be done with it in context of threads. 

But there is one thing definitely wrong in your code and that is constructing TMemo in the context of the background thread. VCL controls can only ever be used from the main thread. Because how that memo control is connected to the Python engine sounds like it is TComponent based. Just being TComponent based does not necessarily mean that it cannot be used in the background threads, but in such cases all related components must belong to the same thread. 

If we know that TMemo must be used in the main thread, then if you connect it to the Python engine implies that such configured Python engine also must be constructed and used from the main thread.

Just because you managed to "make it work" without obvious error (if you ignore the leak), does not mean that this kind of setup actually works.

9 hours ago, Tom F said:

Any suggestions on how to eliminate the flash.

Flicker in VCL Styles can have many root causes depending on the controls used and their settings. It is hard to tell what exactly causes it here and how to fix it. It seems like here it is caused by a form (or whatever control you have as a main container) clearing its canvas with white (default) color before painting dark style. 

First step would be to check all combinations of various settings on both form and other controls (especially if you have another container control that host other controls) (you may want to create simple test project for that) like ParentBackground, DoubleBuffering,... Also setting explicit background color to the dark one could help but that would also require adjusting StyleElements and removing seClient from them. 

If that does not work then another way would be to start playing with WM_ERASEBKGND messages in a container control (frame or panel) that is not styled. Fix mentioned in RSP-24415 has some issues on Windows 11 and non styled controls like combo boxes (I haven't tested with styled ones) that are not properly refreshed and they need their repainting and erasing tweaked. 

It is possible that you will be able to find some satisfactory fix by simple property changes and figuring out proper combination for that form, but it is also possible that fix would require more complicated hacks. 

27 minutes ago, David Heffernan said:

No it won't. If the constructor fails then an exception is raised before the try / except is active.

Right... my brain is fried...

3 hours ago, Uwe Raabe said:

 

function GetUpDate(Link: string): string;
var
  S: string;
  IdHTTP: TIdHTTP;
begin
  IdHTTP := TIdHTTP.Create(nil);//Form1);
  try
    S := IdHTTP.Get(Link);
    result := S;
  except
    ShowMessage('Could not get VERSION information');
  end;
  IdHTTP.Free;
end;

The exception is handled with a ShowMessage call and after that the code following the end is executed, which calls IdHTTP.Free. Although I also prefer the try-finally approach, this code is perfectly valid, as long as ShowMessage returns as expected.

This code is not 100% correct, though. If constructor fails, Free will be called on an uninitialized reference. In order for it to work properly, IdHTTP should be initialized to nil before calling constructor.

begin
  IdHTTP := nil;
  IdHTTP := TIdHTTP.Create(nil);
  ...

Edit... forget about that.... brain fart... if the exception is raised IdHTTP.Free will never be called.

3 minutes ago, Anders Melander said:

No you don't. I understand why you used it but there are always better ways to solve a problem than using [weak].

 

Contrary to what the documentation states [weak] isn't named named after the type of reference. It's named after the design skills of people who choose to use it to manage references. 🙂

Weak references are integral part of ARC as memory management system. Because ARC was not main memory management system in classic compiler and was just an "add on", most code didn't require them or could use pointers that are equivalent of unsafe attribute.  Support for weak references in classic compiler was added only after they were added on mobile ARC compiler which requires weak references in order to have functional memory management system.

Proper coding pattern for ARC memory management would indeed require using weak attribute in the similar circumstances (code) OP had. In other words, if we would rewrite RTL and VCL to follow ARC rules then the OP original code would be the correct one, and we would not need nor have TComponent notification system.

48 minutes ago, gioma said:

I need the weak parameter, because in real use they are two components that refer to each other.

Weak attribute only works on interface references, not on object references. In your case, with TComponent reference, compiler just ignores weak attribute. 

If you are having trouble managing objects memory, then using interfaces is a bad option as using them requires even more knowledge. Yes, they can manage memory for you, but there is so much fine print attached I wouldn't recommend it as a starting point.

Your drawing object list class also has some unnecessary code as it inherits from generic TObjectList that already automatically handles all that. I have simplified your code and added some basic workflow.

uses
  System.Generics.Collections;

type
  TDrawingObjectType = (doChild, doBackground);

  TDrawingObjectsList = class; //Forward Declaration

  TDrawingObject = class
  private
    FObjectType: TDrawingObjectType;
    FDrawingObjectsList: TDrawingObjectsList;
  public
    constructor Create(AObjectType: TDrawingObjectType);
    destructor Destroy; override;
    property ObjectType: TDrawingObjectType read FObjectType write FObjectType;
    property DrawingObjectsList: TDrawingObjectsList read FDrawingObjectsList;
  end;

  TDrawingObjectsList = class(TObjectList<TDrawingObject>)
  public
    procedure DrawDraw(ACanvas: IskCanvas);
    procedure LoadDrawingObjects;
  end;

  procedure BuildList(List: TDrawingObjectsList);

var
  Root: TDrawingObject;

implementation

constructor TDrawingObject.Create(AObjectType: TDrawingObjectType);
begin
  FDrawingObjectsList := TDrawingObjectsList.Create(True);
end;

destructor TDrawingObject.Destroy;
begin
  FDrawingObjectsList.Free;
  inherited;
end;

procedure BuildList(List: TDrawingObjectsList);
var
  Item: TDrawingObject;
begin
  List.Clear;

  // build list

  Item := TDrawingObject.Create(doChild);
  // set Item data
  // ...
  // Add Item to List
  List.Add(Item);

  Item := TDrawingObject.Create(doChild);
  // set Item data
  // ...
  // Add Item to List
  List.Add(Item);
end;


initialization

  Root := TDrawingObject.Create(doBackground);
  BuildList(Root.DrawingObjectsList);

finalization

  Root.Free;

end.

FDrawingObjectsList list will handle lifetime of any drawing object added to the list, so you don't have to deal with them. Please note that you cannot add same drawing object to multiple lists as it will be released twice, causing exception. Next, FDrawingObjectsList belongs to its drawing object instance and as such it should be constructed in constructor and destroyed in destructor of TDrawingObject. Nobody else should free that list nor assign anything to its reference there is no reason for it to be writeable property. Constructing/destroying part is the simplest and safest possible, there are other more complex coding patterns that involve lazy initialization, but they require more code and there is more room for bugs. Unless you need to preserve memory as much as possible, you don't need to deal with more complicated code. And if needed you can easily add that later on.

There is additional mistake in your destructor declaration. You are missing override directive. Without it destructor will never be called and you will have memory leaks.

I have created global Root object and populated it in initialization section. You don't have to make it a global, this is just to illustrate how to work with Root object and how to populate its data. BuildList takes TDrawingObjectsList as a parameter and populates existing list without constructing new one. if the list already has some items it will be cleared - you can omit clearing the list if you know you will populate it only once, or you can clear it in outside code before calling BuildList.

I am not going to comment on the interface part as it would be a really long write.

7 minutes ago, frankie1 said:

i had take a look at what System.Net offers reading the embarcadero docs you posted. However i couldn't find an example to start with.

There are some examples here: https://docwiki.embarcadero.com/RADStudio/Sydney/en/Using_an_HTTP_Client

    var
      Client: THTTPClient;
      Response: IHTTPResponse;
    begin
      Client := THTTPClient.Create;
      try
        Response := Client.Get('https://httpbin.org/get');
        ...
      finally
        Client.Free;
      end;

7 minutes ago, frankie1 said:

Considering i have to log in to:

https://a.b.com/loginform.aspx

username: user1

password: password1

please can you show me what code should i use to retrieve the 'hello user1' after the succesfull login?

This is not just https. Specific example would depend on what kind of request server expects and what kind of response it returns.

If the server uses REST architecture there is another set of classes work with REST https://docwiki.embarcadero.com/RADStudio/Sydney/en/Tutorial:_Using_the_REST_Client_Library_to_Access_REST-based_Web_Services

You don't have to use OpenSSL. Delphi has own HTTP classes that support https protocol through OS provided security layer. Unless you have other specific reasons to use Indy, it is easier to use RTL classes (or components) from System.Net namespace than using Indy and fiddling with OpenSSL

https://docwiki.embarcadero.com/Libraries/Sydney/en/System.Net

10 minutes ago, Attila Kovacs said:

If you have such a strong reference, you don't need the whole hocus-pocus. It will be the point where the object should be released regardless of whether it's an interface or not.

Of course. If the reference and the object is valid longer than the threads using such object, then there is no problem. But there is no need for FreeAndNil then, too.  Nor locking.

But that only works if that specific point can be moved outside threads or if you can use locking mechanism within threads. I am saying "can use locking" because locking is not always viable solution. You might have situation where such point is not fixed, and shared object instance through multiple interface references may be better choice as the object is valid as long as some thread is using it and you don't need to use locks.

16 minutes ago, Attila Kovacs said:

I mean, the idea using FreeAndNil instead of locking.

Obviously it is developer's fault for using inappropriate code in some scenario. I don't know from where you have pulled "blaming the language" because nobody did that.

8 hours ago, Attila Kovacs said:

What interface references and automatic memory managed instances allow you to do is to have multiple, unshared, references to
a shared object instance. And as long as some of those references is alive, that object instance will be alive and valid, too.

But this is a different program as freeing randomly shared objects (Beginning of the story). You can do the same with a shared counter and atomic inc/dec without interfaces.

Not to mention, that re-referencing that interface (creating new unshared references) has to be made in a tricky way, creating first a new, temporary reference to be sure its refcnt won't reach 0 at the same time.

Yes, it is a different "program". as automatic memory management requires not only changes in class declarations, but also in how they are used. fixing existing code and making it thread-safe, requires locking mechanism. There are other ways to ensure that the reference is valid while it is being used, but shared counter and atomic increments and decrements are not sufficient to achieve that.

I mentioned interfaces, because automatic memory management allows different approach to a problem, and in some scenarios leads to simpler and more maintainable code, that does not require locking mechanisms that blocks other threads. Whether such approach is viable in particular scenario is another question, but we were not talking about exact code and without exact code you can only talk in very broad and general terms.

When creating new reference, you don't do that from the background thread, you take existing strong reference and assign it to another one from the context of a thread that holds that strong reference so it cannot become invalid while you are assigning it to another reference. Then you pass that new reference to a new thread. As long as you have strong reference that will not be niled by any other thread, you can safely create new ones from that one. What you cannot do is assigning and nilling the same reference from different threads. No need for any kind of tricks. 

Another solution for initial problem, would be not calling FreeAndNil from any thread and waiting for all threads to finish before instance is released. But again, without knowing exact code, it is hard to say what is the best and proper solution.

8 hours ago, Attila Kovacs said:

I'm also not sure, what this has to do with FreeAndNil(). Looks like somebody wanted to take a shortcut but did not work out and culprit is the language?

I am not sure what you mean with the last sentence.

While we wait for the video, I have written some additional explanation about FreeAndNil thread safety https://dalijap.blogspot.com/2022/07/freeandnil-debate-thread-safety.html

1 hour ago, Lars Fosdal said:

Anyways, ARC has gone the way of the Dodo. Free(AndNil) it is.

Yes, I know. Situation where we had two different memory models that required slightly different coding patterns was unbearable. 

When people belittle ARC as memory management model, they tend to miss the fact that the problem with full ARC memory management was not the ARC as memory management model, nor the compiler itself, but the existing code that was not coded according to ARC rules. Hence the whole DisposeOf nightmare we had. Also, every memory management mode has good sides and bad sides. Each model is better for some use cases and worse for other. 

21 minutes ago, Lars Fosdal said:

Just .DisposeOf's... and the reoccuring questions about why objects doesn't self-destruct (after you have intentionally or unintentionally made references that keeps it alive.

DisposeOf is needed solely for compatibility with existing code. Without maintaining compatibility and coding purely according to ARC rules, there would be no need for DisposeOf, too. 

17 minutes ago, Attila Kovacs said:

I can't see any valid reason not to use FreeAndNil() yet, just complaints about not being able to read others code by ppl. who are well known for being ARC advocates.

I gave my reasons and arguments. I never said anyone has to agree with me.

One thing is for sure. With ARC compiler that wouldn't have to care about compatibility with existing codebases there would be no Free nor FreeAndNil. 

12 minutes ago, David Heffernan said:

Your argument about intent is the closest that I have ever seen to a cogent argument on this subject. But I still don't buy it.

Fair enough.

12 minutes ago, David Heffernan said:

In your argument, if you see code that sets references to nil then that indicates this "optional lifetime" pattern, for the sake of inventing a name.  And your point is that if you set the reference to nil every time you destroy an instance, then you can't distinguish the optional lifetime pattern from the two more common lifetime patterns, let's call them "owned lifetime" where the object is destroyed in its owner's destructor, and "local variable lifetime" where the object is created and destroyed in a local method, and the reference is a local variable.

 

The thing is though, when you see FAN in owned lifetime and local variable lifetime, it's unmistakeable what the lifetime is.  You never think, oh, I wonder if this might be optional lifetime.  That's why I don't buy the argument, and why I've never experienced the issues you describe.

I am saying that in some situations lifetime is beyond obvious, no matter what you use.  Both in locally constructed instances and in instances with larger scope, regardless whether their lifetime is dynamic or not.

But I have seen plenty of more complex code, where lifetime is not as clear cut and where there are thousand lines of code involved with multiple references (to different objects that are functionally intertwined). So FreeAndNil in destructor can make a difference between intended behavior and bugs, And the FreeAndNil might solve one bug only to make another one appear. Being able to categorize references based on intended behavior can make untangling such code an easier task. 

12 minutes ago, David Heffernan said:

What I have experienced though is the pain of using a stale reference.  That pain is eased if the reference is nil.  And yes, we can use debug MM to write over memory when an object is destroyed, but I don't want that to happen in my release builds.  And sometimes bugs only emerge in release builds.

One thing is for sure. For you I don't doubt that you do know what particular code does. I may think that you are needlessly using FreeAndNil, but it will not make me think that your code is possibly bug ridden because you have no idea what you are doing. Unfortunately, that is generally not true and in plenty of code I have seen using FreeAndNil in "wrong" place was nothing comparing to the other coding horrors and bugs it contained. 

1 hour ago, David Heffernan said:

My code uses FAN rather than free as a rule, and I don't recognise the problem you describe. That's my practical experience. 

I am not that smart... I need all the help I can get...

I cannot say for sure how would it work for my own code, which I know the best, because I don't use FreeAndNil everywhere. But this has definitely been a problem for me when reading other people's code. Or maybe that is just because it was overall not the best code ever written...

35 minutes ago, David Heffernan said:

This makes no sense at all. If you instead use Free and retain a stale reference to a destroyed object, then other code simply cannot know that the reference is stale. 

I am talking about scenarios where object instances are lazily constructed or can be nil at some point for other reasons.

For instance, think of list of children that is nil if there are no children and is only valid when child is added, but is also destroyed when last child is removed. In such case you would check if the instance is nil and construct the list before adding an new child. If you remove some child and there are no children left in the list, you would then call FreeAndNil on such list. If you are doing some processing on the children, you would just check if the list is nil and skip processing code in such case, because there are no children that need to be processed. So being nil is valid value for such reference and it can become nil at various stages during its owning instance lifetime. 

This is drastically different coding pattern than having some list which will be constructed in constructor and destroyed in destructor and will be valid reference in the rest of the code. For such references I would use Free and not FreeAndNil.

This is the distinction of the code intent I am talking about. If you use FreeAndNil everywhere then you lose the intended design and usage. Of course, tehre are other ways to detect how the instance should be used, but nothing is a problem in simple code and scenarios. In more complex code losing the intent is much harder to deduct and it can mean the whole world of difference. I would say that you can solve that problem with leaving comments in the code, but I have yet to see such comments in code that indiscriminately use FreeAndNil. 

If I want to detect stale reference access I would use other tools, instead of relying on FreeAndNil.  

35 minutes ago, David Heffernan said:

Object destroyed by owner, either from another instance's destructor, or in a method with try finally lifetime. FAN is just a debugging guard against use after free. 

 

Or a reference to an instance whose lifetime is dynamic. Perhaps created and destroyed multiple times by its owner. Then nil is used to indicate the state that the object is currently not in existence. 

Yes, those are the two scenarios. And for the first one there are better tools, especially since FreeAndNil only nils one reference and you may have other stale ones. Yes, I have occasionally used FreeAndNil for detecting stale pointer access, but only as temporary debugging tool and because I was too lazy to setup FastMM for some reason (read not important project). In other words, I would replace it with Free when the code is fixed (I don't think I have ever used it for debugging purposes more than few times)

35 minutes ago, David Heffernan said:

It seems bizarre to me that anybody could advocate carrying around stale pointers. 

I am not advocating that anywhere. That is why I am using Free (except for the second scenario with dynamically constructed objects), because I am not touching stale pointers, so using FreeAndNil is pointless. 

28 minutes ago, Vincent Parrett said:

Zero, I know it's not thread safe.

I was kidding. I don't even have to look at your code to know it is not thread-safe  

28 minutes ago, Vincent Parrett said:

It wasn't just freeandnil that solved the issue (if only). Just another tool in the box, which I used then removed. 

Like I said, using it for debugging purposes when something is off, and for some reason you cannot pinpoint the problem with other tools is fine.

28 minutes ago, Vincent Parrett said:

I still don't get why people get upset about FreeAndNil - I have much bigger and more important issues with delphi to deal with (like actual bugs in the ide/compiler/rtl/vcl) rather than worry about something so minor.  

Problem is not in using FreeAndNil. It has its use case (beyond debugging purposes). The problem is using it everywhere, and then it obscures the intent of the code. When you FreeAndNil, your code is sending the different message that when you just Free. If you FreeAndNil that means such reference can be nil at any point and all code using such reference also needs to deal with scenario where reference is nil, either constructing new instance if the reference is nil, or skipping operations on nil reference. And that is drastically different code than one dealing with reference that cannot be nil.

6 hours ago, Vincent Parrett said:

I suspected it could be a reference counting issue (since I use interfaces a lot), so started sprinkling some FreeAndNils around the code (in destructors) - suddenly those random av's turned into nil pointer exceptions that were much less random. That confirmed to me that the issue was with accessing objects already free'd. The problem turned out to be that I was using weak references where I really needed strong references - a 70hr week of frustration turned into a 4 line change 🤦‍♂️

 

One question... you say you are using interfaces and then that calling FreeAndNil pointed you into right direction. Why are you calling Free (or FreeAndNil)  on reference counted instance? If reference counting is disabled then the concept of strong and weak references is not applicable.

5 hours ago, Vincent Parrett said:

I saw that, but I'm not using the built in Weak stuff - this is in XE7 Win32 before weak was available - I'm using my own weak reference library - it's not as convenient (need to use a base class) but has worked well for me for the last 12 years or so. I still take a hard reference from the weak and check that for nil rather than relying on the .IsAlive property when using it in threads.

How much you pay me to prove your Weak library is not thread safe 

Main problem with FreeAndNil supporters is that it is treated as silver bullet and it is not. Like @Stefan Glienke said, there are better tools for the job. And in multithreading scenario all bets are off. Of course, if you suspect something is out of order then you can add some help like calling FreeAndNil for debugging purposes. Been there, done that. But sprinkling them all over the code, just in case? Sorry, but that is hard no. It just pollutes the code and gives you false sense of security, because it will nil just one reference and all others will turn into dangling pointers.

In other words, if FreeandNil can save me 70 hours of work in one case, it would still cost me way more work hours, when I have to read all code with FreeAndNil wondering whether there is a real purpose behind that call or it is there just for luck.

1 hour ago, Der schöne Günther said:

Oh, in that case, absolutely. Guess I should watch it.

I mean, it can always turn out to be mindless "use this... no use that..." fighting. But without knowing the facts around object construction and destruction process, it is impossible to have an argument based discussion. 

24 minutes ago, Der schöne Günther said:

What did I miss? What I understand is that

1. It is not part of the language, it is just a regular method from the standard library
2. Earlier versions did have an untyped parameter and let you pass in things that were not descendants of TObject

If that is all, how can you even have an opinion on that? What is there to talk about? Can somebody get me up to date with just one or two sentences?

There is plenty to talk about. It is not just about FreeAndNil, but about constructing and destructing object and all the fine print around that process. While on the surface this topic seems simple enough and that there is nothing to talk about, it is rather complex topic and many developers, even experienced ones, easily forget about some very important facts around this topic that can cause serious issues in applications.

14 hours ago, ringli said:

The whole thing is already running in a simple thread, but sometimes it seems to hang a bit. Therefore I wanted to test if there is a better or simpler solution. I always tend to solve everything too complicated.

If you are already running it in a thread, then adding event bus on its own will not solve your problem. 

Even bus would help decoupling your code that is doing the search from the code that is showing the results, but it will not run faster. On the contrary. Since you are adding results to the UI, that part needs to run in the main thread. So you would use TThread.Queue or TThread.Synchronize. Event bus would call the same code in order to run event handler in the main thread. But when you do that you are calling it directly. Sending message through event bus runs a bit more code. You need to create message (event) which will cost some time, depending on the event type and the data it passes. Then sending message alone will lock the collection of subscribers, iterate through that collection, locate appropriate subscriber and then it will invoke event handler for that subscriber. And when iteration is completed it needs to unlock collection. 

So decoupling comes at some price. In most cases that price is well worth paying, because bus overhead is very small comparing to other code that runs, but it will definitely not run faster than some code that is directly wired and invoked.

Why is your code having problems, is hard to say without seeing the code. Also, when you are searching on disk, performance will be tied with the content of the disk, and its hardware characteristics, as well as the whole system. For instance, if you try to access physically damaged part of the disk OS call may hang on such spot for minutes. 

13 hours ago, ringli said:

The project seems to be very interesting.

Thanks!

13 hours ago, ringli said:

So far I have no experience with an event bus to be able to estimate whether I can use the library in my small hobby projects.

 

For me (and I'm sure for others) a few small practical examples would be very instructive. It would be great if you could upload some examples to the github repository.

I will try to add some. I will need some time to prepare some meaningful examples that can show potential use cases.

Event bus is a messaging system. Delphi already has basic event bus implementation in System.Messaging https://docwiki.embarcadero.com/CodeExamples/Alexandria/en/System.Messaging_(Delphi) You can also look at the examples there as those use cases apply to my event bus, too.

Main difference is that System.Messaging is not thread-safe and you can only use it to send messages in the context of the main thread. If you want to send messages across multiple threads you need a thread-safe event bus, like NX Horizon. Because, it is thread-safe, it also has some additional features like dispatching events (messages) asynchronously in the background thread.

Maybe the easies way to explain what is event bus is comparing it to a Button OnClick event handler. When user clicks a button code in the OnClick event handler will run. main difference (besides multithreading support) is that with button and its event handler there is usually deeper connection and there is direct link with the button and its event handler. For instance if you click Help button on some form, you would want to open Help window from its OnClick event handler. But in that case your form with button needs to know about help form. If you have many forms that need to open help form will create tight coupling between all those forms and help form. 

With event bus, you can declare TOpenHelp event type and then you can subscribe some code to such event type. In your forms with help buttons, you would still need OnClick event handler, but instead of directly opening help form from that OnClick event you can send a message to event bus that TOpenHelp event happened. And then subscribers to that event (there can be more than one) will receive it and run the appropriate code in associated subscription event handler. This way your forms don't need to know about your help form, and code handling your help form does not need to know from where TOpenHelp came from. 

Event type also serves two purposes. Its type tells that particular event happened, and its content (event can be any automatically managed or value type) is used to pass additional data. for instance if the TOpenHelp is integer type, you use it to store and pass help page number depending on which help button is clicked and then you can open help on particular help page. 

Another example would be downloading some files in the background thread and then sending TDownloadCompleted event from that thread with some data about particular download and then subscribers can handle and do whatever they need to do with that data. Process it further, show it to the user, or anything else.