Phillip J. Eby wrote: [...]

There's also one twist that I haven't sorted out yet: "Importing" guarantees that a parent module 'foo' will have a 'bar' attribute for the 'foo.bar' module, if 'foo.bar' is lazy. It does this by registering a callback, ideally *before* any other callback is registered for 'foo' or 'foo.bar' that would look at 'foo.bar'. I don't see how to maintain this condition in a world where import callbacks can be registered independently.

I've moved the PyImport_NotifyModuleLoaded() call to import_submodule(). It (should) guarantee that the hooks for a parent package is called before the hooks for its children are called. I've analyzed the code carefully enough to be sure but all unit test results are on my side. On other words "import a.b.c" fires the hooks for "a", then "a.b" and at last "a.b.c". I could also modify imp.notify_module_loaded to accepts the module name as string ("a.b.c."). If the module is provided by name (e.g. "a.b.c.") rather than by object it makes sure that the hooks for "a", "a.b" and "a.b.c" are called in the right order.

Bleah. All of the above isn't really a good explanation of the problem. Let me try to simplify it:

* Lazy importing needs to guarantee that foo.bar = sys.modules['foo.bar'], when callbacks for 'foo.bar' execute (in case they refer to foo.bar)

* To do this, it registers a callback that sets foo.bar = sys.modules['foo.bar'], and does not actually register any foo.bar callbacks until 'foo' is really imported (and thus foo.bar gets set by that callback)

Would the modification fulfill your needs if imp.notify_module_loaded("foo.bar.baz") call the hooks for "foo", "foo.bar" and "foo.bar.baz" in that order?

In the case of the PEP, it's harder for me to figure out what happens, because you might not have any lazy modules around, and the foo.bar issue would then not come up. You also have the possibility of a problem where a lazy import callback occurs in 3rd party code, while callbacks are occurring from the import machinery. (Which means that the notification API should probably set the hooks entry to None while it's running, so that if it's called from inside a hook, it will not double-run the hooks, and new hooks registered while hooks are running will get run immediately as they are encountered, instead of getting added to the list.)

The initial design used to set the hooks to None *after* the hooks were called. I removed code yesterday because I thought it's not required. Today I've re-added the checks for Py_None. I'm not setting the hooks to Py_None before the hook are called. Christian

Phillip J. Eby wrote:

...

I'm not setting the hooks to Py_None before the hook are called.

Err, make that NOW, not NOT ... stupid typo. I'm NOW setting the hooks to Py_None before the hooks are called.

That's fine, but here's a different catch: are you iterating over the hooks by taking the length before you start? If so, then hooks that are added *while* the hooks are being called back, will never get called, because they'll be added to the end of the list (and you'll never reach the end). Make sure there's a test for that case.

it = iter(self.post_import_registry[name]) self.post_import_registry[name] = None for hook in it: hook(module)

peak.util.imports sets to None after callbacks, but it uses regular list iteration, so hooks can be added to the end of the list while the hooks are still being called.

In my version a hook is immediately called when the the registry value is set to None. When a hook is registered for a module during the execution of the callback then the hook is fired directly and not after the existing hooks are called. Is this a problem for you? module "foo" is loaded: hook1 hook2 -> registers hookX for "foo" hookX is called directly hook3 hook4

An error while running the hooks should also set the hook list to None and discard all the hooks. There isn't any sane way to recover from an error in a post-import hook.

The hooks are set to None even when an exception is raised by a hook. Christian

Phillip J. Eby wrote:

At 11:45 PM 1/10/2008 +0100, Christian Heimes wrote:

...

In my version a hook is immediately called when the the registry value is set to None. When a hook is registered for a module during the execution of the callback then the hook is fired directly and not after the existing hooks are called. Is this a problem for you?

Yes, because it violates the invariant that hooks for a given module are called in the same order that they were registered in.

Please check the changes and the new unit test in r59902 ( http://svn.python.org/view?rev=59902&view=rev ). Are you satisfied with the ordering or do you think I should queue the hooks for already loaded modules? Christian

Phillip J. Eby wrote:

Yes, that's the general idea. But what happens if you are in the middle of firing hooks for 'a', and a new hook for 'a.b.c' is added? What about a new hook for 'a'?

If 'a' registers a new hook for a child of 'a' (e.g. 'a.b.c' or 'a.f') then the new hooks are called with the remaining hooks for 'a.b.c': import a.b.c * hook_a1 * hook_a1 -> registers the hook_ab2 for 'a.b' * hook_ab1 -> registers a hook_aX for 'a' hook_aX is fired immediately * hook_ab2 -- the hook registered by hook_a1

Well, it certainly can (and should) do the same if a module object is provided, since the module has a __name__.

Maybe I should add two methods to imp. One that calls the parent hooks of a module automatically but relies on the existence of the parents and the module in sys.modules. And a second method which calls the hooks for a module object w/o inspecting sys.modules.

Only if you can guarantee that no hook for a submodule is run until all the parent hooks are finished being called *and* that adding new callbacks while callbacks are being run will still call them... *after* any already-added callbacks.

Uhm, now it starts to become a mind bending problem. I may have to queue and delay the registration of new hooks while other hooks are called by the system. If an user registers a callback for 'a' while the callbacks for 'a' are called than the registration is queued and the called after the remaining hooks for 'a' are called. This could probably be implemented by *not* setting the entry to None after I get hold of the iterator but after the end of the iteration. iter() notices when a new element is appended. In the above sample can hook_aX be called after hook_ab1 or should it be called after hook_ab2?

In general, if you think something in peak.util.imports isn't required, you're probably wrong. ;)

*g* ok. I'll check your implementation again. Christian

Phillip J. Eby wrote:

*sigh*. We seem to be getting further and further off course, here. The more different you make the semantics of the system, the harder it will be to verify that it's doing the right thing without having real field experience with the new approach.

*sigh*, too. :/ This discussion has neither helping me nor you. Could you please write an unit test or two to show me exactly what my implementation is doing wrong and how you think the callbacks should be called. I know a simple test won't proof the correctness of the implementation but a failing test would at least show the incorrectness. I'm still not sure which way is the correct way in your opinion and I hate guessing what you are trying to explain to me. Christian PS: I've a pending patch that queues new registrations while hooks are processed in PyImport_NotifyModuleLoaded().

Phillip J. Eby wrote:

when_imported('a.b')(func_ab1) when_imported('a.b')(func_ab2)

@when_imported('a') def func_a1(module_a): when_imported('a.b')(func_ab3) notify_module('a.b') # <- this is here to foil trivial implementations

when_imported('a')(func_a2) notify_module('a.b')

This should produce the calling sequence:

func_a1, func_a2, func_ab1, func_ab2, func_ab3.

My implementation calls the hooks in the right order but I had to insert fake modules into sys.path. # insert the modules into sys.modules to fake a 3rd party import a = imp.new_module('a') ab = imp.new_module('a.b') a.b = ab sys.modules["a"] = a sys.modules["a.b"] = ab # notify imp.notify_module_loaded('a.b') Otherwise the code fails to fetch the module objects from sys.path and an exception is raised.

1. notification is only done once for a given module, ever, even if the notification function is called more than once, even if it's called during notifications for that module

The latter is not yet implemented. notify_module('a.b') currently fails with a recursion limit exceeded. It's on my todo list.

2. notifications for a child module/package may not begin until the notifications for the parent package have begun

This is already implemented for notification by name but not for notification by module object. It's also on my todo list.

3. two registrations for the same module must always be invoked in the same order as they were registered, even if some of the registrations are done during notification.

I'm pretty but not yet absolute sure that my implementation guarantees the last invariant. I've to go through my code several times and play through use cases.

In order to implement these invariants, you will have to have a way to know whether notifications have been begun for a given module. In peak.util.imports, the module objects effectively keep track of this, although they don't have a specific flag. For the Python implementation, you could add a __notified__ field to module objects, and implement the notify function thus:

That's a nice idea to fix the recursion issue with nested notifications.

Of course, __notified__ would actually be a structure slot, rather than an attribute, so as to avoid any attribute lookup issues with module subtypes (e.g. lazy modules).

Sure, I can use a slot for PyModule and its subclasses. Unfortunately other implementations of lazy imports are adding a proxy object which is not a subclass of PyModule. I've to use an attribute if not isinstance(mod, moduletype).

The register function would simply append a hook to the entry in post_import_hooks if it's not None, or call the hook otherwise.

My code queues up new hooks while a sequence of hooks is processed. It makes sure that hooks for a parent aren't called in the middle of a child's hook chain. notification_in_progress = False queue = [] def register_hook(hook, name): if notification_in_progress: queue.append((hook, name)) return hooks = sys.post_import_hook_register(name, None) if hooks is None: module = sys.modules.get(name, None) if modules: sys.post_import_hook_register[name] = None hook(module) else: hooks = [] sys.post_import_hook_register[name] = hooks hooks.append(hook) def notify_loaded(mod_or_name): notification_in_progress = True try: ... finally: notification_in_progress = False while queue: hook, name = queue.pop(0) hook(sys.modules[name])

With this implementation, I could make a version of peak.util.imports that did its own lazy modules, but used the base system for all the hooks.

Very good! Christian

Phillip J. Eby wrote:

At 10:14 PM 1/15/2008 +0100, Christian Heimes wrote:

...

My code queues up new hooks while a sequence of hooks is processed. It makes sure that hooks for a parent aren't called in the middle of a child's hook chain.

Notice that that's not necessary with the notification algorithm I gave, since the list in post_import_hooks suffices as a queue. So, just as in peak.util.imports, the registration code doesn't need to know whether callbacks are being run; it only needs to know whether they're *finished*.

Are you sure your proposed algorithm and output match for the test case? I'm confident I got it right in C but I'm getting a different output. Without the extra imp.notify_module_loaded('a.b') in func_a1(mod):: ['func_a1', 'func_a2', 'func_ab1', 'func_ab2', 'func_ab3'] With the extra imp.notify_module_loaded('a.b') in func_a1(mod):: ['func_a1', 'func_ab1', 'func_ab2', 'func_ab3', 'func_a2'] I can't see how your implementation results in the first output when func_a1() calls the notification method.

Of course, both the notification and registration functions must hold the import lock to prevent a race condition where one thread adds a hook to the list after another thread has just finished iterating over it and is about to replace the list with None. At least, they have to if they're executing any Python code that might cause the GIL to be released. The callbacks will release the GIL, of course, but the registration code probably doesn't... well, it will if it calls the hook, and ISTM that the hooks should always execute with the import lock held, even if they're fired at registration.

I'm aware of the implications and my code already uses the lock. The PyImport_NotifyLoaded() method excepts to be called with the importer lock acquired. So I'm locking the importer lock in imp_notify_module_loaded(). The PyImport_RegisterPostImportHook() method does the locking before it accesses sys.modules and sys.post_import_hooks. Christian

Phillip J. Eby wrote:

I guess it's not right then. ;-) Though I shouldn't make fun, since it turns out that my code sketch was not a correct translation of peak.util.imports. (See below.)

*grrrrr* I spent more than hour to find my error ...

That is, module.__notified__ has to be set *before* the recursive notification call. This effectively happens in peak.util.imports now, except that __notified__ isn't an explicit attribute, just a side effect of other module state changes.

It's done. Your proposed test cases passes together with my tests. The ref leak tests don't show a single missing reference. Christian

Phillip J. Eby wrote:

Sorry about that - as I said, __notified__ is very much an implicit thing in peak.util.imports.

And I believe I've also mentioned a lot of times how hard it is to get this stuff right... :)

*hehe* Indeed, you did! I should have been warned. :)

Congrats! Now all we need to do is get the authors of other lazy import/export/whatever systems to chime in with whatever additional invariants *they* might need... ;-)

Oh, no! You are kidding me ... But yes that's "all" we need to do. And I've to update the patch and explain why the implementation does all the crazy stuff. Christian

At 07:22 PM 1/10/2008 +1000, Nick Coghlan wrote:

Christian Heimes wrote:

...

A module is successfully loaded '''''''''''''''''''''''''''''''

The import machinery checks if sys.post_import_hooks contains post import hooks for the newly loaded module. If hooks are found then the hooks are called in the order they were registered with the module instance as first argument. The processing of the hooks is stopped when a method raises an exception. At the end the entry for the module name is removed from sys.post_import_hooks, even when an error has occured.

Doesn't the module remain in post_import_hooks, only mapped to None to indicate that any hooks should be run immediately?

It should be, yes.