Jump to content

Dalija Prasnikar

Members
  • Content Count

    1046
  • Joined

  • Last visited

  • Days Won

    91

Dalija Prasnikar last won the day on February 16

Dalija Prasnikar had the most liked content!

Community Reputation

1327 Excellent

Technical Information

  • Delphi-Version
    Delphi 11 Alexandria

Recent Profile Visitors

The recent visitors block is disabled and is not being shown to other users.

  1. Dalija Prasnikar

    Anonymous methods as interfaces

    You don't understand. They are implemented as interface based class. Without that implementation they wouldn't work at all. It is just that compiler hides that fact from us and automatically creates everything needed.
  2. Dalija Prasnikar

    Anonymous methods as interfaces

    There would be no reason to change it, short of removing anonymous methods. Early design is discussed here http://blog.barrkel.com/2008/07/anonymous-method-details.html
  3. Dalija Prasnikar

    Delphi and "Use only memory safe languages"

    Memory leaks as such are indeed not relevant for this discussion, but accessing invalid memory is the focus of this conversation and manual memory management opens up such possibility.
  4. Dalija Prasnikar

    Delphi and "Use only memory safe languages"

    I think that you don't fully grasp the difference between forgetting to free an object instance which will cause a memory leak and accidentally triggering reference counting mechanism which will trigger premature destruction of an object which is still in use. For providing different implementation for classes by using interfaces, it is not enough to add interface to the class, you need to change all signatures from class to interface. This is not backward compatible. Also, TComponent does not implement reference counting, it is disabled for almost all use cases, and it works under very specific scenarios. Not to mention that TComponent is one of the main reasons why full ARC compiler was removed. It is completely incompatible with ARC. TXmlDocument is an outlier. I beg to differ. Original code had a memory leak because of wrong assignment, but such leak is detectable and if you follow the leak you can find issue in the code. If you had IStrings, you would not have a leak, but you would still have a bug in your code because there is still a wrong assignment somewhere. Anything can be source of bugs, just the bugs and issues you have will be different. https://github.com/dalijap/code-delphi-mm/blob/master/Part5/SmartPointer/Smart.dpr procedure UseStrings(Strings: TStrings); begin Writeln(Trim(Strings.Text)); end; procedure SmartPtr; var sl: ISmartPointer<TStringList>; begin sl := TSmartPointer<TStringList>.Create(); sl.Add('I am inside automanaged StringList'); UseStrings(sl); end; I think the above code is simple enough. The name SmartPointer is rather long and it can be easily shortened. If you would want to have the same functionality as the above code with interfaces while preserving backward compatibility then you would have to write something like: procedure UseStrings(Strings: TStrings); begin Writeln(Trim(Strings.Text)); end; procedure SmartPtr; var sl: IStrings; begin sl := TStringList.Create(auto); sl.Add('I am inside automanaged StringList'); UseStrings(TStrings(sl)); end; Which also shows why you cannot insert different implementation as you need to typecast back to TStrings if you want to preserve backward compatibility. I assume this is the ticket https://quality.embarcadero.com/browse/RSP-36609 I fully agree with the closure. Adding interfaces to TStrings for the sole purpose of having automatic memory management would only add burden to that class hierarchy while achieving nothing that cannot be done by smart pointers. What Bruneau Babet suggests is the right course of action, adding memory management from the within opens possibility for wrong usage and accidental errors. Using smart pointers (the examples from C++ and Rust are in line with above smart pointer example) is the way to go. Now, there is no universal smart pointer support built in the RTL, adding that would be more meaningful and more versatile solution than adding interfaces to handpicked classes.
  5. Dalija Prasnikar

    Delphi and "Use only memory safe languages"

    First, that is not proper description how TXmlDocument memory management works - I understand it is possible that you just wanted to convey what you meant by adding interfaces to RTL classes (and that served the purpose), but I need to add precise explanation of how it works because that class is one of the most incorrectly used ones when it comes to memory management. The deciding factor in how the TXmlDocument memory is managed is not in how it is stored (reference type, although it does have a play in how the code will behave), but whether you have constructed the instance with nil owner or not. This will set FOwnerIsComponent field and depending on that field the instance will be automatically managed with reference counting or not. If the owner is nil, you must store created instance in interface reference and if the owner is not nil, then you may store it in either interface reference or object reference, but the memory will not be managed by interface reference if you do that and you must make sure that interface reference does not outlive owner. Following console application demonstrates this. XmlDocInterafce and XmlDocComponent show correct usage, the other two are incorrect ones that can cause trouble depending on the code (you can get lucky and avoid issues, but that does not mean that such usage is correct and will not blow when you least expect it) program XMLDocMM; {$APPTYPE CONSOLE} {$R *.res} uses System.SysUtils, System.Classes, Winapi.ActiveX, Xml.Win.msxmldom, Xml.XMLDoc, Xml.XMLIntf; var Owner: TComponent; procedure UseDoc(Doc: IXMLDocument); begin Doc.LoadFromXML('<abc>aaa</abc>'); end; procedure XmlDocInterface; var x: IXMLDocument; begin x := TXMLDocument.Create(nil); UseDoc(x); Writeln(x.IsEmptyDoc); end; procedure XmlDocInterfaceLeak1; var // this will create leak as reference counting is not properly triggered x: TXMLDocument; begin x := TXMLDocument.Create(nil); x.LoadFromXML('<abc>aaa</abc>'); Writeln(x.IsEmptyDoc); end; procedure XmlDocInterfaceLeak2; var x: IXMLDocument; begin x := TXMLDocument.Create(Owner); UseDoc(x); Writeln(x.IsEmptyDoc); // this will create memory leak as memory is not managed by interface reference, but owner Owner.RemoveComponent(TComponent(x)); end; procedure XmlDocComponent; var x: TXMLDocument; begin x := TXMLDocument.Create(Owner); try UseDoc(x); Writeln(x.IsEmptyDoc); finally x.Free; end; end; procedure XmlDocComponentSelfDestruct; var x: TXMLDocument; begin x := TXMLDocument.Create(nil); try UseDoc(x); // x will be destroyed here Writeln(x.IsEmptyDoc); // accessing destroyed instance finally // Invalid Pointer Operation x.Free; end; end; begin ReportMemoryLeaksOnShutdown := True; CoInitialize(nil); try Owner := TComponent.Create(nil); try XmlDocInterface; XmlDocInterfaceLeak1; XmlDocInterfaceLeak2; XmlDocComponent; XmlDocComponentSelfDestruct; finally Owner.Free; end; except on E: Exception do Writeln(E.ClassName, ': ', E.Message); end; CoUninitialize; end. So if you want to add interfaces to some RTL class and change its memory management behavior, you cannot do that just by adding interface to its declaration and required reference counting methods and let the reference type decide the memory management. I mean in simple scenario you could, but such code would be very fragile. Following code works correctly when it comes to memory management, but if the reference counting is in any way triggered by second example (Test2) it would auto destroy the object instance. procedure Test1; var i: IInterface; begin i := TInterfacedObject.Create; end; procedure Test2; var o: TInterfacedObject; begin o := TInterfacedObject.Create; o.Free; end; So, if you want to have such class that can have its memory managed in dual way, you need to handle that through some additional parameter during instance construction (or through some special value of existing parameter, like it was done with TXmlDocument) that would prevent accidental reference counting from destroying the object prematurely. And then you would also have to pay attention on how you store such instance and that reference type matches passed parameter and associated memory management. On top of that, you would need to declare interface for the instance public API because otherwise you would not be able to use any of the object's intended functionality. With TXmlDocument the whole functionality is already interface based, so exposing that through outer wrapper is trivial. In real life scenarios, if you need to automatically manage some object instance, there are better ways to do that (through some kind of smart pointer implementations or similar) than by forcing interfaces on those base classes. I had such base class in my library as part of some base class hierarchy but I don't use that ability except in few rare cases. I though I would use it more, but adding it on such low level was a mistake as I almost never used it across span of 20 years I had it. Classes that had interfaces defined I used through interface references anyway, there was never a need to switch to manual memory management for those, and occasional usage of automatic memory management for other classes that didn't have declared interfaces was not enough to justify the implementation in the base class. Main reason was that using such class with dual memory management without having its specific interface was more complicated than using smart pointers or just writing few extra lines of code and doing it manually. I am not saying that sometimes such dual memory management is not useful, but that such instances are rare and adding such thing on majority of RTL classes would be waste of time and effort.
  6. Dalija Prasnikar

    Delphi and "Use only memory safe languages"

    How would that be backward compatible? Classes that use reference counting have completely different memory management and require slightly different handling than ones that are not reference counted. Every line of code that would use such changed classes would have to be inspected and changed. This would be like having ARC compiler all over again, only without half of required changes being automatically implemented in the compiler. You cannot force ARC on existing code that is not written with ARC in mind. That does not work. It may work in some simple scenarios, but then when you get to slightly more complex code is all falls apart.
  7. Dalija Prasnikar

    Delphi and "Use only memory safe languages"

    Yes, that was the core problem. In other words, the ARC compiler alone on the existing codebases and Delphi component system which required DisposeOf would not be any more memory safe than code using non ARC compiler. Some loopholes that would be plugged by having automatic instance initialization and lifetime management, but DisposeOf opens other different loopholes. So, if we would go that route, then all Delphi code (including RTL, VCL and FMX) would have to be written from scratch, which makes ARC compiler not really viable solution after all.
  8. Dalija Prasnikar

    Delphi and "Use only memory safe languages"

    I hate to be the one saying this... but ARC compiler was a step in that direction. You cannot have memory safety without automatic memory management.
  9. It was my comment about the server creation which seemed unnecessary. This was a wrong assumption, and the server is needed for the authorization process. However, there are still some things in your code that could be improved. I will start with simple things. You are have plenty of unnecessary nil checks. If you want to return a nil or some field instance if it is assigned, then you can just return the value in the field as if the field is not assigned its default value will be nil. (Note, when it comes to local variables, they must be explicitly initialized to nil, as their default value will be random, unlike for object fields) So following code can be replaced function TServer.ServerLog: TStringList; begin Result := Nil; if Assigned(FServerLog) then begin Result := FServerLog; end; end; with function TServer.ServerLog: TStringList; begin Result := FServerLog; end; Next, you are constructing FServerLog in the constructor, so the whole nil check is pointless anyway because during the server instance lifetime it will be valid, assigned instance. In cases where it would not be a valid instance. If you would have a case where some function could return nil instance, you would have to always check for nil, before using such object. Free method can be safely called on nil instances, so you don't have to check if instance is assigned before calling Free. Following would be fine: destructor TServer.Destroy; begin FServer.Free; FServerLog.Free; inherited; end; Because server uses server log, I would also reverse order of construction and destruction and create server log first, and then the server, also server would be destroyed first, and then server log. You have similar code in other places and it can be simplified, too. Now, back to the actual problem you have been asking. First of all, TShopeeAuthorizator is inherently connected with TCatcherServer. It would make sense to create server in TShopeeAuthorizator constructor and destroy it in its destructor. This would also prevent memory leak you have if the FieldsReadyHandler is never called. This would also remove rather ugly Sender.Free code. It is not that it cannot be used in that way, but commonly such code can be replaced with better constructs. Next it would be better to move parts of the TShopeeContext.Authorize inside the TShopeeAuthorizator. This would also remove the need to have global AuhorizationDone event. Make it a field in TShopeeAuthorizator and create it in its constructor, and destroy in the destructor. The whole waiting for even loop will then be moved inside TShopeeAuthorizator.AuthorizationRequest which would be converted to function returning Boolean to detect successful authorization. procedure TShopeeContext.Authorize; var Authorizator: TShopeeAuthorizator; begin // Request Authorization; Authorizator := TShopeeAuthorizator.Create(FDataHolder, FHost, FAPI_Key, FPartnerID, 8342); try if Authorizator.AuthorizationRequest(45000) then begin ShowMessage('Autenticado com Sucesso'); ShowMessage(FDataHolder.Code + ' ' + FDataHolder.EntityID); end else raise Exception.Create('Authorization Timed Out'); finally Authorizator.Free; end; end; Additionally, you can also make TShopeeContext.Auhorize a Boolean function that would merely return the value returned by AuthorizationRequest. This would also move error handling to the higher level code which can then have better control of how to handle success and failure. You can also add event handlers FOnSuccess and FOnFailure to TShopeeContext so the outside code can hook to them. In such case Authorize does not need to be a function (but you can still leave it like that, so your code would have more flexibility. TShopeeContext = class private FOnAuhorizeSucess, FOnAuthorizeFailure: TNotifyEvent; ... published property OnAuthorizeSucess: TNotifyEvent read FOnAuhorizeSucess write FOnAuhorizeSucess; property OnAuthorizeFailure: TNotifyEvent read FOnAuthorizeFailure write FOnAuthorizeFailure; end; function TShopeeContext.Authorize: Boolean; var Authorizator: TShopeeAuthorizator; begin Authorizator := TShopeeAuthorizator.Create(FDataHolder, FHost, FAPI_Key, FPartnerID, 8342); try Result := Authorizator.AuthorizationRequest(45000); if Result then begin if Assigned(FOnAuthorizeSucess) then FOnAuthorizeSucess(Self); end else begin if Assigned(FOnAuthorizeFailure) then FOnAuthorizeFailure(Self); end; finally Authorizator.Free; end; end; Of course, you can move both success and failure into single event handler, but then you would need to define handler that would have additional parameter.
  10. Dalija Prasnikar

    What new features would you like to see in Delphi 13?

    Imagine how slow would compilation be with those optimizations turned on. I hope than newer LLVM could be faster, but I wouldn't keep my hopes up https://discourse.llvm.org/t/if-llvm-is-so-slow-is-anything-being-done-about-it/75389
  11. Dalija Prasnikar

    Delphi and "Use only memory safe languages"

    Delphi is not memory safe language. But, at the end it all depends on the kind of code you write.
  12. Dalija Prasnikar

    What new features would you like to see in Delphi 13?

    Forget about that. While there is always a possibility that speed may be slightly improved, slowness is a feature of LLVM backend.
  13. Dalija Prasnikar

    Delphi IDE alternate registry location issue

    The best would be to contact support https://www.embarcadero.com/support choose "Registration and Installation" issues there.
  14. Each permission affects different functionality so it is only natural that there will be different rules because the impact and the consequences of abuse can be vastly different. But guessing what the actual rules are, beyond what is written in official documentation would be just playing guessing games.
  15. You should remove any permissions you don't need for application functionality. Applications are analyzed when posted on Play Store and having more dangerous permissions defined can put your application under the magnifying glass and you may need to give additional explanations or comply to some other policies than regular applications that don't use those permissions. Some unneeded permissions can even cause your application to be rejected from Play Store until you fix your submission. Official documentation is usually the best starting point for being up to date with requirements https://developer.android.com/guide/topics/permissions/overview and https://support.google.com/googleplay/android-developer/answer/9888170
×