Yesterday I found an interesting issue.

Yesterday I found an interesting issue.
A lazy instanced shared object is declared in the implementation section of a unit A and protected by an access function which use a critical section to avoid double creation.  The critical section was instantiated in the initialization section of Unit A.  The ClassA types call the shared function during instantiation.

unit A references units X, which use unit B, that defines ClassB as a descendant from ClassA in unit A.

In Unit B, I added instantiation of a ClassB in the initialization section.

Boom!

Unit A's init section had not been called before Unit B's init section!
The critical section object was nil!

This has to be a bug, or?

I solved it by putting the critical section into a different shared unit used by both A and B.

Comments

  1. I have found that the exact sequence in which initialization code happens can no longer be relied on, unlike older delphi versions

    ReplyDelete
  2. That is pretty severe.  Is there a QC?

    ReplyDelete
  3. I forgot - it seems that the finalization also is in the same mis-order - hence broken.

    ReplyDelete
  4. I don't think it's a bug per se, more a change, I'm sure I read it in some release notes somewhere

    ReplyDelete
  5. Afaik it depends if the unit is in the interface or implementation uses. If possible prefer class constructors to initialize things.

    From http://docwiki.embarcadero.com/RADStudio/XE5/en/Programs_and_Units#The_Initialization_Section:
    For units in the interface uses list, the initialization sections of the units used by a client are executed in the order in which the units appear in the client's uses clause.

    ReplyDelete
  6. Code used by Unit A which is not ready to run when Unit B calls the code - not a bug? 

    http://docwiki.embarcadero.com/RADStudio/XE5/en/Programs_and_Units

    says: "The initialization section is optional. It begins with the reserved word initialization and continues until the beginning of the finalization section or, if there is no finalization section, until the end of the unit. The initialization section contains statements that are executed, in the order in which they appear, on program start-up. So, for example, if you have defined data structures that need to be initialized, you can do this in the initialization section.
    For units in the interface uses list, the initialization sections of the units used by a client are executed in the order in which the units appear in the client's uses clause."

    In my app, UnitA is before in UnitB in the app's uses list.

    ReplyDelete
  7. So Unit A -> Units X -> Unit B -> Unit A?

    You don't mention whether each dependency is in the interface or implementation section for each unit.

    If they were all in the interface then it wouldn't compile due to Delphi's circular dependency restriction.

    The documentation only guarantees the initialization order for units in the interface section. In fact it is bolded, which leads me to think that units that are only used in the implementation section either don't get initialized at all or the order cannot be assumed. I'm inclined to think it's the former.

    Without the circular dependency, the units referenced in the interface section would be initialized in reverse order of the dependency chain. So for:
    A -> B -> C

    Initialization would be ordered:

    C, B, A

    If the dependencies were displayed as a graph, the leaf nodes would be initialized first, followed by lowest branch nodes, then so on and so forth.

    ReplyDelete
  8. From the top of my head - Unit B uses Unit A in the interface section, Unit X uses Unit B in the implementation section., and Unit A uses Unit X in the interface section. 

    In the .dpr file uses clause, Unit A is before Unit B.  Shouldn't that take precedence?

    I'm not sure if I'll spend time on an SSCCE since I found a workaround, but from this point on, Paul Foster's rule of can't trust the order goes.

    ReplyDelete
  9. If this is the case (and assuming units in the implementation section uses clause are not initialized) the order of initialization would be:
    X, A, B

    In any case when I have something I want to be a singleton I usually write a function prototype in the interface section that returns an interface:

    function MySingleton: ISingleton;

    Then in the implementation section I write:

    var
      ASingleton: ISingleton;

    function MySingleton: ISingleton;
    begin
      if not Assigned(ASingleton) then
        ASingleton := TSingleton.Create;
      Result := ASingleton;
    end;

    I usually don't bother with cleanup because it'll be freed when the last interface reference goes out of scope. I tend to favor reference passing over singletons though.

    Just realized this is probably what you're doing but you need it to be thread safe. Hence the critical section.

    ReplyDelete
  10. Practical experience Lars Fosdal I used to maintain code which worked fine, the unit initialization was done the order I designed, one delphi upgrade and the whole lot fell apart, not only was the order not guaranteed but the timing changed too, I used to rely on it happening before form creation but that stopped too. I found it best to treat these sections as lazy initialization that happens at some point during startup as if it's in a multithreaded run, it's harder to code but always worked

    ReplyDelete
  11. Ouch. I'm pretty sure we have code which relies on the initialization order. Paul Foster, do you recall which version that introduced this break?

    ReplyDelete
  12. Not really, it was somewhere between 5 and 2007 I think, I'm not involved any more and it's been a couple of years

    ReplyDelete
  13. Paul Foster Aaight, we just upgraded from D2010 to XE3 so hopefully should be safe.

    ReplyDelete
  14. Unless something else has been changed :-P

    ReplyDelete
  15. Using a critical section to make singleton creation threadsafe is overkill, TInterlocked.CompareExchange makes it much simple, eg :

    function RegisteredClasses : TDictionary;
    begin
      if _registeredClasses = nil then
      begin
        //thread safe lazy construction
        Result := TDictionary.Create;
        if TInterlocked.CompareExchange>(_registeredClasses,Result,nil) <> Result then
          Result.Free;
      end;
      Result := _registeredClasses;
    end;

    ReplyDelete
  16. Nice trick, and that is essentially the purpose of the function :)

    ReplyDelete

Post a Comment