Skip to content

Commit

Permalink
Add workaround for import endless loop in python loader.
Browse files Browse the repository at this point in the history
  • Loading branch information
viferga committed May 27, 2021
1 parent ad7ed1b commit 9c421eb
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 34 deletions.
84 changes: 50 additions & 34 deletions source/loaders/py_loader/source/py_loader_impl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,15 @@ void py_loader_impl_module_destroy(loader_impl_py_handle_module module)
PyObject_DelItem(system_modules, module->name);
}

// TODO: Sometimes this fails, seems that sys.modules does not contain the item.
// Probably is because of the new import system which is using importlib, but
// it does not seem something problematic although it will be interesting
// to check it out so we are sure there's no leaked memory
if (PyErr_Occurred() != NULL)
{
PyErr_Clear();
}

Py_DECREF(module->name);
module->name = NULL;
}
Expand All @@ -1835,7 +1844,7 @@ void py_loader_impl_handle_destroy(loader_impl_py_handle py_handle)
free(py_handle);
}

int py_loader_impl_load_from_file_module(loader_impl_py py_impl, loader_impl_py_handle_module module, const loader_naming_path path)
int py_loader_impl_load_from_file_path(loader_impl_py py_impl, loader_impl_py_handle_module module, const loader_naming_path path)
{
loader_naming_name name;
size_t size = loader_path_get_fullname(path, name);
Expand All @@ -1852,14 +1861,22 @@ int py_loader_impl_load_from_file_module(loader_impl_py py_impl, loader_impl_py_
goto error_name_create;
}

PyObject *args_tuple = PyTuple_New(1);
PyObject *py_path = PyUnicode_DecodeFSDefault(path);

if (py_path == NULL)
{
goto error_path_create;
}

PyObject *args_tuple = PyTuple_New(2);

if (args_tuple == NULL)
{
goto error_tuple_create;
}

PyTuple_SetItem(args_tuple, 0, module->name);
PyTuple_SetItem(args_tuple, 1, py_path);

module->instance = PyObject_Call(py_impl->import_function, args_tuple, NULL);

Expand All @@ -1875,53 +1892,56 @@ int py_loader_impl_load_from_file_module(loader_impl_py py_impl, loader_impl_py_
}

Py_DECREF(args_tuple);
Py_DECREF(py_path);

return 0;

error_module_instance:
Py_DECREF(args_tuple);
error_tuple_create:
Py_DECREF(py_path);
error_path_create:
Py_XDECREF(module->name);
module->name = NULL;
error_name_create:
return 1;
}

int py_loader_impl_load_from_file_path(loader_impl_py py_impl, loader_impl_py_handle_module module, const loader_naming_path path)
int py_loader_impl_load_from_module(loader_impl_py_handle_module module, const loader_naming_path path)
{
loader_naming_name name;
size_t size = loader_path_get_fullname(path, name);
size_t length = strlen(path);

if (size <= 1)
if (length == 0)
{
goto error_name_create;
// TODO: Handle exception
return 1;
}

module->name = PyUnicode_DecodeFSDefaultAndSize(name, (Py_ssize_t)size - 1);
module->name = PyUnicode_DecodeFSDefaultAndSize(path, (Py_ssize_t)length);

if (module->name == NULL)
{
goto error_name_create;
// TODO: Handle exception
return 1;
}

PyObject *py_path = PyUnicode_DecodeFSDefault(path);
/* TODO: Trying to reimplement the PyImport_Import,
* check the TODO in monkey patch method in the port for more information
*/
/*
PyObject * globals = PyEval_GetGlobals();
if (py_path == NULL)
if (globals != NULL)
{
goto error_path_create;
Py_INCREF(globals);
}
PyObject *args_tuple = PyTuple_New(2);

if (args_tuple == NULL)
{
goto error_tuple_create;
}
module->instance = PyImport_ImportModuleLevelObject(module->name, globals, NULL, NULL, 0);
PyTuple_SetItem(args_tuple, 0, module->name);
PyTuple_SetItem(args_tuple, 1, py_path);
Py_XDECREF(globals);
*/

module->instance = PyObject_Call(py_impl->import_function, args_tuple, NULL);
module->instance = PyImport_Import(module->name);

if (!(module->instance != NULL && PyModule_Check(module->instance)))
{
Expand All @@ -1931,22 +1951,13 @@ int py_loader_impl_load_from_file_path(loader_impl_py py_impl, loader_impl_py_ha
module->instance = NULL;
}

goto error_module_instance;
goto error_import;
}

Py_DECREF(args_tuple);
Py_DECREF(py_path);

return 0;

error_module_instance:
Py_DECREF(args_tuple);
error_tuple_create:
Py_DECREF(py_path);
error_path_create:
Py_XDECREF(module->name);
error_import:
Py_DECREF(module->name);
module->name = NULL;
error_name_create:
return 1;
}

Expand Down Expand Up @@ -1998,8 +2009,13 @@ loader_handle py_loader_impl_load_from_file(loader_impl impl, const loader_namin
for (iterator = 0; iterator < size; ++iterator)
{
/* Try to load the module as it is */
if (py_loader_impl_load_from_file_module(py_impl, &py_handle->modules[iterator], paths[iterator]) != 0)
if (py_loader_impl_load_from_module(&py_handle->modules[iterator], paths[iterator]) != 0)
{
if (PyErr_Occurred() != NULL)
{
PyErr_Clear();
}

/* Otherwise, we assume it is a path so we load from path */
if (loader_path_is_absolute(paths[iterator]) == 0)
{
Expand Down
6 changes: 6 additions & 0 deletions source/ports/node_port/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ describe('metacall', () => {
assert.notStrictEqual(py_encode_basestring_ascii, undefined);
assert.strictEqual(py_encode_basestring_ascii('asd'), '"asd"');
});
it('require (py submodule dependency)', () => {
// Require the 'op' submodule from 'fn' Python package
const { foldr, call } = require('fn.op');

assert.strictEqual(foldr(call, 10)([s => s * s, k => k + 10]), 400);
});
it('require (rb)', () => {
const cache = require('./cache.rb');
assert.notStrictEqual(cache, undefined);
Expand Down
16 changes: 16 additions & 0 deletions source/ports/py_port/metacall/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import os
import sys
import json
import inspect # TODO: Remove this, check the monkey patching

if sys.platform == 'win32':
from metacall.module_win32 import metacall_module_load
Expand Down Expand Up @@ -168,6 +169,21 @@ def generate_module(handle_name, handle):
# Generate the module from cached handle
return generate_module(handle_name, handle)

# This breaks a recursion loop when trying to load py files
# The problem is basically that PyImport_Import in the loader tries to
# load as module first, so it jumps to this method again. Probably
# we should find a more efficient solution, for example reimplementing
# PyImport_Import (which I failed) or checking if the port is present
# and the import is monkey patched, so in this case we should populate
# the real python __import__ from the port into the loader, so we can
# avoid jumping always to the patched import, this is a workaround
# that must be removed because it is dirty and slow (TODO)
if extension == 'py':
current_frame = inspect.currentframe()
call_frame = inspect.getouterframes(current_frame, 2)
if call_frame[1][3] == 'metacall_load_from_file' or call_frame[1][3] == 'metacall_load_from_memory':
return ImportError('MetaCall could not import:', name)

if metacall_load_from_file(extensions_to_tag[extension], [name]):
handle = find_handle(name)
if handle != None:
Expand Down

0 comments on commit 9c421eb

Please sign in to comment.