On Thu, Nov 01, 2012 at 06:50:30PM -0400, George Bosilca wrote:
> On Nov 1, 2012, at 16:18 , Nathan Hjelm <hjelmn_at_[hidden]> wrote:
> > On Thu, Nov 01, 2012 at 04:07:32PM -0400, George Bosilca wrote:
> >> Nathan,
> >> Here is a quick question regarding the topi framework.
> >> - The mca_topo_base_output is opened unconditionally in topo_base_open.c:62
> >> - with your patch, mca_topo_base_output is closed conditionally in topo_base_close.c:46, but only in case mca_topo_base_components_opened_valid and mca_topo_base_components_available_valid are NULL. However, mca_topo_base_output is set to -1 in all cases right after.
> >> Why is that so?
> > mca_base_components_close closes the output if the third argument is NULL. So in this case calling opal_output_close after mca_base_components_close will result in a second call to opal_output_close.
> Indeed, the behavior of the mca_base_components_close seems quite weird to me, as it lack of consistency.
> - the symmetric function (mca_components_open) doesn't open the output stream
> - the stream is close but the corresponding variable is not set to a meaningful value (-1)
Exactly. There is more inconsistency between the two calls as well. mca_base_components_open calls OBJ_CONSTRUCT on the component list but mca_base_components_close does not call OBJ_DESTRUCT. Because of this we don't always OBJ_DESTRUCT the component list because some frameworks don't do it.
I was going to address this second inconsistency with another patch but now seems like a good time to get a see if anyone has an opinion about how this should be fixed. I can think of two simple fixes:
1) Since mca_base_components_open calls OBJ_CONSTRUCT should mca_base_components_close call OBJ_DESTRUCT?
2) Should the caller be responsible for both the OBJ_CONSTRUCT and OBJ_DESTRUCT calls?
> - it force us to have one specific output for each framework
This isn't the case at the moment since frameworks can call opal_output_close on any extra output streams. It would be better if frameworks have t close all open output streams using opal_output_close instead of using mca_base_components_close. If we want to change the semantics of mca_base_components_close I can redo this patch. Anyone have an opinion on this?