From doc [1], when create_module returns a non-module instance, m_methods, m_traverse, m_clear, m_free must be NULL. But actually in the codes, only m_traverse, m_clear, m_free are checked and emitting consistent errors. If m_methods is NULL, it will fail in [2] and emit an inconsistent misleading argument error. And what's more confusing is, in [3], it says "regardless of type, the module's functions are initialized from m_methods, if any", which I think conflicts with the codes and doc.

[1] https://docs.python.org/3.6/c-api/module.html#c.Py_mod_create
[2] https://hg.python.org/cpython/file/tip/Objects/moduleobject.c#l300
[3] https://www.python.org/dev/peps/pep-0489/#post-creation-steps

I think the intended behaviour here is that which was documented in the PEP: that m_methods should still work based on ducktyping for reading a __name__ attribute and setting the method attributes, even if the result of Py_create_mod isn't a module type object.

However, it isn't currently working due to the PyModule_Check call in PyModule_GetNameObject, which is in turn called from PyModule_AddFunctions. The Py_mod_create docs then reflect that limitation of the implementation.

The cleanest way to refactor and fix this that comes to mind would be to make static _get_object_name() and _add_methods_to_object() functions in moduleobject.c (which omit any strict type checks), and then call those from PyModule_GetNameObject and PyModule_AddFunctions with the explicit typecheck.

That way we don't have to support this for arbitrary third party code calling in to the C API, while still allowing it for the specific case of objects returned from Py_mod_create.

The cleanest way to refactor and fix this that comes to mind would be to make static _get_object_name() and _add_methods_to_object() functions in moduleobject.c (which omit any strict type checks), and then call those from PyModule_GetNameObject and PyModule_AddFunctions with the explicit typecheck.

That's one solution. How about loosing PyModule_GetNameObject's constraint? Let it accept non-ModuleType objects? Actually without the constraint of PyModule_GetNameObject, PyModule_AddFunctions can handle non-ModuleType objects.

Loosening the constraint on PyModule_GetNameObject would indeed work, but it means the code still has a readability problem: the convention in the C API is that officially ducktyped APIs use the PyObject_* prefix, or one of the other abstract protocols (PyNumber_, PyMapping_, etc), rather than a concrete type name like PyModule_*. Relying on folks to "just know" that these particular APIs deliberately don't enforce the type constraint is a recipe for future confusion, even if it's documented that way.

Such a change also has potential ripple effects on other implementations that emulate the C API, and hence isn't something I'd be comfortable with changing in a maintenance release, whereas fixing the implementation to match the PEP could be done for the next 3.5.x update.

Nice point. I'll write a patch to fix this these days.

Hi! I'm on a tight schedule this week, so I'm not looking into this in detail. But please let me know if you need any help and I'll raise the priority.

Thanks Petr. I'd appreciate it if you are willing to review the patch.

Upload a patch to fix this, along with tests and doc updating.

But here is something different. In PEP-489, it is explicitly stated that "Py_mod_create slot is not responsible for setting import-related attributes specified in PEP-451 (such as __name__ or __loader__ ) on the new module". So when an object(even ModuleType instances) is returned, it's __name__ attribute is not set and we can't rely on it (which means we can't even use PyModule_GetNameObject). I then use the name attribute of the spec. Looking forward to feedback.

The patch looks good to me, and the relevant contributor licensing is in place, so I'll be applying this shortly :)

New changeset 913268337886 by Nick Coghlan in branch '3.5':
Issue bpo-27782: Fix m_methods handling in multiphase init
https://hg.python.org/cpython/rev/913268337886

New changeset fb509792dffc by Nick Coghlan in branch 'default':
Merge bpo-27782 fix from 3.5
https://hg.python.org/cpython/rev/fb509792dffc

Thanks for the bug report and the patch!

These kinds of collaborative interactions are my favourite aspect of open source participation :)

Thanks for your work too, Nick! :) Active reply from the core devs always gives me more motivation to open source.

Xiang, if multi-phase initialisation is an area you're interested in, it occurs to me you may also want to take a look at Petr's proposal to provide efficient access to global module state from methods of extension types: https://mail.python.org/pipermail/import-sig/2016-August/001065.html

With the 3.6 feature freeze deadline imminent it's looking like that will be deferred to 3.7, but it's still a significant improvement that should encourage greater use of the newer more submodule and reloading friendly approach to C extension module initialisation.

Thanks for your notice, Nick. :) Of course I am interested. I'll start following import-sig and reading Petr's good idea.