#353 Improve start up code ordering issues

[Philip Kendall]

Fuse's start up code has some slightly hairy ordering issues - certain <foo>_init() functions have to be called before others with no real idea as to the ordering. Make this better.

There's also the slightly embarassing "FIXME FIXME 20030407: really do this soon". 13 years later, we should either fix it or remove the comment :-)

[Sergio Baldoví]

And the exit code too. Leaving dependencies aside, I feel that the desirable order for <foo>_init() and <foo>_end() functions should be LIFO, i.e., last initialised should be the first to end.

[Philip Kendall]

Sergio: agreed. I made some good progress on this last night, but just didn't get round to committing it - I think the infrastructure I'm putting in place should allow LIFO for the exit functions as well.

[Philip Kendall]

Sergio: see [r5648]: all registered modules are now unwound in reverse order to that in which they were initialised.

[Sergio Baldoví]

Looks pretty good. Now that init() and end() functions share the same graph of dependencies, we should double-check dependencies to avoid any potential problem.

Some comments...

printer_end() depends on settings, previously was called before settings_end(). Also depends on machine module and there isn't any guarantee that it is invoked before machine_end().

rzx_end() depends on settings and machine, therefore should depend on STARTUP_MANAGER_MODULE_MACHINE. Calling setting_end() as early is a pain, it would be nice to use the startup manager, calling settings_end() after peripherals, before memory.

There are modules with <foo>_end() functions without <foo>_init() functions (pokemem_end, svg_capture_end). It would be easy allowing NULL init() funcions and use the startup manager only for end() functions.

There is an ARRAY_SIZE macro with type checking in compat.h. Maybe we should use that in dependency_count for consistency.

The fake GLib implementation lacks g_array_sized_new(), g_array_remove_index_fast(), g_array_append_val(), g_array_index().

I wonder if there is need to create the infrastructure/ directory just for one file.

[Philip Kendall]

Sergio, thanks for looking at this :-)

[r5651] adds a dependency on the machine module to the printer and RZX modules which will ensure they get invoked before machine_end().

[r5652] adds use of ARRAY_SIZE.

For the other points:

• For now, would just moving settings_end() after startup_manager_run_end() fix things? I agree in the medium term it would be nice to get this the startup manager, but I'm currently wondering about the best way to deal with the "drop root privileges" stanza: almost every module could declare a dependency on that, but that's pretty ugly - but we really do want as little code as possible run before we drop root privileges.
• Agreed on having modules with NULL init functions. Just a matter of writing the code :-)
• Fake GLib functions: g_array_index and g_array_append_val are defined as macros (see make-perl.c). The others we'll have to write - shouldn't be hard, just boring...
• I think there are probably other things that can go in the infrastructure directory - e.g. mempool.c. In any case, I'd like to avoid the root directory being used as a dumping ground for "stuff which doesn't fit anywhere else", and I didn't think the startup manager fit into any of the existing directories.

[Sergio Baldoví]

For now, would just moving settings_end() after startup_manager_run_end() fix things?

Probably yes, except for MacOSX. I wonder if this comment still applies:

/* also required before memory is deallocated on Fuse for OS  X where
 settings need to look up machine names etc. */

If both modules are managed inside the startup manager, I think ports/UIs might customize the dependencies of some modules.

In the worst case, settings_end() and memory_end() could be called after startup_manager_run_end()?

I agree in the medium term it would be nice to get this the startup manager, but I'm currently wondering about the best way to deal with the "drop root privileges" stanza: almost every module could declare a dependency on that, but that's pretty ugly - but we really do want as little code as possible run before we drop root privileges.

Looks acceptable calling popular modules before run_startup_manager() to avoid cluttering.

[Fredrick Meunier]

Probably yes, except for MacOSX. I wonder if this comment still applies:

/* also required before memory is deallocated on Fuse for OS  X where
 settings need to look up machine names etc. */

Yes, still does :)

[Philip Kendall]

Fake GLib implementations of g_array_sized_new() and g_array_remove_index_fast() done in [r5655].

[Philip Kendall]

Fred (and others): [r5664] moves setting_end into the startup manager, along with a conditional dependency for OS X. I think this should work, but I have no way of testing it... somebody let me know if it needs changing.

[Fredrick Meunier]

Thanks Phil, the patch is looking very good.

t's only the "Fuse for Mac OS X" version (soon to be Fuse for macOS?!) that needs the dependency to memory for settings and no define comes to mind to pick that out to switch behaviours between SDL Fuse on the Mac vs Fuse for OS X.

I suggest that settings_register_startup just has the following and I'll maintain the STARTUP_MANAGER_MODULE_MEMORY dependency version as a patch on the "Fuse for Mac OS X" version.

  startup_manager_module dependencies[] = {
    STARTUP_MANAGER_MODULE_SETUID,
  };
  startup_manager_register( STARTUP_MANAGER_MODULE_SETTINGS_END, dependencies,
                            ARRAY_SIZE( dependencies ), NULL, settings_end );
 }

[Philip Kendall]

Thanks Fred, done in [r5666] - I've left the line in there but commented out so that there's no temptation to merge things together and cause issues for the OS X port if we ever get settings_init() into the startup manager.

[Fredrick Meunier]

Much obliged :)

[Philip Kendall]

I've done some final tidy-ups (removing the debug printfs and the FIXME comments in fuse.c) and think this one is now pretty much ready to merge. I'll try and do that on Friday night unless anyone objects so we can get this in for the next release.

[Philip Kendall]

And merged :-)

[Sergio Baldoví]

Merged in [r5670].