Delayed initialization for extension models

[trustednstaller]

Can support be added (it is trivial) to delay the initialization of extension models using the DbgModelClientEx.h client library? There is mention of it within the source but an implementation is lacking. I would consider this critical to the support the library adds as often times you want to initialize data models under certain conditions especially when you have many data models within a single extension. It would be easiest to have this support builtin directly to the provided interfaces already with perhaps a metadata property of "IsDelayed" etc. as well

[trustednstaller]

e.g.,

namespace Debugger::DataModel::ProviderEx {

	struct DelayedExtensionModel : ExtensionModel {
		bool m_initialized = false;                   // tracks whether we've done final initialization
		std::function<void()> m_initFunc = nullptr;   // captures constructor-time registrations for delayed usage

		//----------------------------------------------------------------------------
		// DelayedExtensionModel(): contructors
		//
		DelayedExtensionModel() : ExtensionModel() {}

		template<typename... TArgs>
		DelayedExtensionModel(_In_ ClientEx::Metadata && metadata,
													_In_ TArgs&&... registrations)
			: ExtensionModel(metadata)
		{
			// Capture the registrations in a lambda for later use
			m_initFunc =
				[this, regs = std::make_tuple(std::forward<TArgs>(registrations)...)]() mutable {
					std::apply(
						[this](auto&&... unp) {
							CompleteExtensionModelInitialization(std::forward<decltype(unp)>(unp)...);
						},
						regs
					);
				};
		}
		
		template<typename... TArgs>
		DelayedExtensionModel(_In_ TArgs&&... registrations)
			: ExtensionModel()
		{
			m_initFunc =
				[this, regs = std::make_tuple(std::forward<TArgs>(registrations)...)]() mutable {
					std::apply(
						[this](auto&&... unp) {
							CompleteExtensionModelInitialization(std::forward<decltype(unp)>(unp)...);
						},
						regs
					);
				};
		}
		
		//--------------------------------------------------------------------------
		// DelayedModelInitialize()
		//
		// No-parameter version: use the registrations captured by the constructor
		// if they exist. If we've already initialized, throw an error.
		void DelayedModelInitialize() {
			if (m_initialized) {
				throw std::runtime_error("DelayedExtensionModel is already initialized!");
			}

			if (m_initFunc) {
				m_initFunc();
				m_initFunc = nullptr; m_initialized = true;
			} else {
				throw std::runtime_error("No registrations were captured for delayed init!");
			}
		}

		// DelayedModelInitialize()
		//
		// Parameterized version: let the user supply fresh registrations 
		// at the time of calling DelayedModelInitialize. This bypasses any 
		// previously captured regs in m_initFunc.
		template <typename... TArgs>
		void DelayedModelInitialize(TArgs&&... registrations) {
			if (m_initialized) {
				throw std::runtime_error("DelayedExtensionModel is already initialized!");
			}
			this->CompleteExtensionModelInitialization(std::forward<TArgs>(registrations)...);
			m_initialized = true; m_initFunc = nullptr;
		}
		
	protected:
		virtual void InitializeModel() = 0; // users override this instead of relying construction to the constructor

	}; // DelayedExtensionModel
} // Debugger::DataModel::ProviderEx

[trustednstaller]

Another useful addition would be a better merge between Registration Records and Extension/TypedInstanceModels. They are clearly interlinked and the separation makes it very tedious (and error prone) to how to initialize what. There should just be model classes specific to each supported registration record or some compile-time logic that can deduce the record type. It is just very verbose to repeatedly initialize a particular registration record of some kind and you don't even know what it is unless you've given it an appropriate name.

[wmessmer-msft]

@trustednstaller A few questions here...

First, may I ask what you see as the benefit here (e.g.: DelayedExtensionModel) versus just constructing the ExtensionModel derived class itself upon demand...? There's nothing that prevents you from constructing an "ExtensionModel" derived class at a later point in time than extension initialization.... the extensions it provides come into life at construction time and disappear at destruction time...

As to the "merge between registration records and extension/typed instance models", while I'll grant that a single record is the most common type of extension, there are times where you want to apply multiple registration records to a given extension (e.g.: extend something and publish your own extensibility point). That, unfortunately, doesn't work well in a "one model class per registration record" paradigm.

[trustednstaller]

@wmessmer-msft Thanks for the follow up!

So yes, you're right in the sense one could expose something like a data model method to construct an ExtensionModel given some runtime criteria and invoke this when they want to load that extension model but why the need for this (effecitvely) wrapper? I think it makes more sense to support it outright in the ExtensionModel and TypedInstanceModel abstractions themselves. There's also mention about "delayedInitialization" so I also assumed there were plans to introduce this at some point anyway:

    // ExtensionModel():
    //
    // Construct an extension model and have it register against the data model in the manners specified by
    // the arguments.  Such must be registration records such as TypeSignatureRegistration, etc...
    //
    // If delayedInitialization is true then InitializeModel must be used in the derived constructor
    //
    ExtensionModel(_In_ ClientEx::Metadata && metadata) : BaseDataModel(metadata)
    {
    }

    ExtensionModel() : BaseDataModel(ClientEx::Metadata())
    {
    }

Here we see default constructors that are effectively useless as one cannot actually initialize the extension model at some point later e.g., CompleteExtensionModelInitialization is a private method. Although InitializeModel is mentioned in the comment it doesn't actually exist in the code.

The real justification to have it, imo, is the added flexibility and control in how one chooses to control their extension models. I don't see the need to add wrappers around this when the logic to add to the abstractions themselves is minimal and has no cost.

There are many instances especially when you have a plugin that attempts to deal with multiple debug targets where you want to automate the initialization of what you need. It is much simpler to have all your extension models already prepared and just add if-else logic in your extension startup that instantiates one model over another e.g., when the debug target is a live kernel debugging session you only want some set of your data models. When it's a particular crash dump, you want others.

To my other point, the vast majority of cases (like you mentioned) are single records. What I am saying is there is an unnecessary overhead in having to know that you have to create an extension model of registration record type "x". It's extremely verbose in every case. We know up front that we're dealing with registration records as it pertains to extension models. Why not simplify the intent by having something like: (*cut out the delayed initialization logic I included for brevity)

	struct NamedModelParentExtensionModel : ExtensionModel {
		std::wstring parentName;
		
		NamedModelParentExtensionModel() : ExtensionModel() {}
		NamedModelParentExtensionModel(
			NamedModelParentArgs const &args)
		: ExtensionModel(),
			parentName { args.ParentName },
		{}
... ... 
	}; // NamedModelParentExtensionModel

	struct NamespacePropertyParentExtensionModel : ExtensionModel {
		std::wstring moduleName;
		std::wstring namespaceName;
		std::wstring propertyName;
		
		NamespacePropertyParentExtensionModel() : ExtensionModel() {}
		
		NamespacePropertyParentExtensionModel(NamespacePropertyParentArgs const& args)
		: ExtensionModel(),
			moduleName    { args.ModelName },
			namespaceName { args.NamespaceName },
			propertyName  { args.PropertyName },
		{}

... ...

This makes the intent much more clear outright for the vast majority of cases. Again, we are creating registration records yet upon initialization we pass the registration record(s) as arguments to the extension model. But we're really after creating registration records so the ask is to have the client library expose the registration records as the interface to work with rather than 1) ExtensionModel 2) RegistrationRecords(s) parameters.