Re: [boost] [review] Formal Fast-Track review of the Boost.Functional/Factory library begins today

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2007-12-18 10:43:58

Hello Emil,

thanks for your comments.

Emil Dotchevski wrote:
> This is not a full review, just a few comments:
> 
> The supplied documentation mentions nothing about the lifetime of a factory.

A factory (in this context) is just a function object.

> 
> There are two features that I would need from a factory system like this.
> 
> First, the ability to register and unregister factories. 

A registry is intentionally not provided as the requirements typically 
differ greatly between projects. Although this submission makes it 
trivial to write one from scratch,

    map< string, boost::function< favorite_smart_ptr<T>(args) > >

to begin with.

> I would
> suggest explicit registration that returns a shared_ptr to the factory
> function, while keeping weak_ptr to it. This way, if the factory
> serves a type that is defined in a DLL, it can be automatically
> unregistered when the DLL is unloaded.

Something like this

     // We're in DLL.CPP here

     namespace {
         shared_ptr< function< plugin*() > > sentinel;
     }

     void plugin_entrypoint(plugin_registry r)
     {
         sentinel.reset(r.register_plugin(
             "my_plugin", factory<my_plugin*>()));
     }

     // Also note: The code is untested and unpolished -
     // it's provided for communication purposes only.

?

Now that I look at some code it seems like it might work but it's 
certainly not the be-all end-all of factory registries, 
'plugin_registry' is still a trivial thing and returning that 
'shared_ptr' seems sorta weird (though it admittedly saves us some work).

Further, automatic unloading won't work portably with some ABIs.

> Second, I would prefer the factory table to not be global, instead its
> type can be documented so it is up to the client code to decide if
> they want it to be global or not.

If I wanted to provide a registry, I'd certainly agree here.

> Finally, it's worth mentioning that in the presence of DLLs using
> type_info directly is unsafe, since dereferencing a pointer that
> points a type_info object obtained from a DLL causes a crash if the
> DLL is unloaded.

Yeah, accessing unloaded memory (or even running unloaded code) seems a 
bad idea in general not only with 'type_info' objects ;-)...

Regards,
Tobias