Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/python asyncio #826

Closed
wants to merge 6 commits into from
Closed

Conversation

88Ocelot
Copy link

Fixes for python asgi

@alejandro-colomar
Copy link
Contributor

Apart from the code comments, could you please detail in the commit messages what this does, and especially why we want it? I don't have context about this change. However, the code changes look something reasonable; thanks for the work!

@alejandro-colomar alejandro-colomar self-assigned this Jan 16, 2023
@88Ocelot
Copy link
Author

its the fix for python 3.10-3.11.
#815

alongside latest python patch releases were made breaking change for lower level asyncio api.
python/cpython#93453 (comment)

Also seems like using asyncio api get_event_loop for main thread was related to this issue
#560

Comment on lines +202 to +325
} handlers[] = {
{ "create_task", &ctx_data->loop_create_task },
{ "add_reader", &ctx_data->loop_add_reader },
{ "remove_reader", &ctx_data->loop_remove_reader },
{ "call_soon", &ctx_data->loop_call_soon },
{ "run_until_complete", &ctx_data->loop_run_until_complete },
{ "create_future", &ctx_data->loop_create_future },
};

loop = NULL;

asyncio = PyImport_ImportModule("asyncio");
if (nxt_slow_path(asyncio == NULL)) {
nxt_unit_alert(NULL, "Python failed to import module 'asyncio'");
nxt_python_print_exception();
goto fail;
}

event_loop_func = "get_loop";
runner_class = "Runner";
runner_ref = PyDict_GetItemString(PyModule_GetDict(asyncio),
runner_class);
if (nxt_slow_path(runner_ref == NULL)) {
nxt_unit_alert(NULL,
"Python failed to get '%s' from module 'asyncio'",
runner_class);
goto fail;
}

runner = PyObject_CallObject(runner_ref, NULL);
if (nxt_slow_path(runner == NULL)) {
nxt_unit_alert(NULL, "Python failed to call 'asyncio.%s'",
runner_class);
goto fail;
}
ctx_data->runner = runner;
loop_ref = PyObject_GetAttrString(runner, event_loop_func);
loop = PyObject_CallObject(loop_ref, NULL);
if (nxt_slow_path(runner == NULL)) {
nxt_unit_alert(NULL, "Python failed to call 'Runner.%s'",
event_loop_func);
goto fail;
}
for (i = 0; i < nxt_nitems(handlers); i++) {
obj = PyObject_GetAttrString(loop, handlers[i].key);
if (nxt_slow_path(obj == NULL)) {
nxt_unit_alert(NULL, "Python failed to get 'loop.%s'",
handlers[i].key);
goto fail;
}

*handlers[i].handler = obj;

if (nxt_slow_path(PyCallable_Check(obj) == 0)) {
nxt_unit_alert(NULL, "'loop.%s' is not a callable object",
handlers[i].key);
goto fail;
}
}

obj = PyObject_CallObject(ctx_data->loop_create_future, NULL);
if (nxt_slow_path(obj == NULL)) {
nxt_unit_alert(NULL, "Python failed to create Future ");
nxt_python_print_exception();
goto fail;
}

ctx_data->quit_future = obj;

obj = PyObject_GetAttrString(ctx_data->quit_future, "set_result");
if (nxt_slow_path(obj == NULL)) {
nxt_unit_alert(NULL, "Python failed to get 'future.set_result'");
goto fail;
}

ctx_data->quit_future_set_result = obj;

if (nxt_slow_path(PyCallable_Check(obj) == 0)) {
nxt_unit_alert(NULL, "'future.set_result' is not a callable object");
goto fail;
}

Py_DECREF(loop);
Py_DECREF(asyncio);

*pdata = ctx_data;

return NXT_UNIT_OK;

fail:

nxt_python_asgi_ctx_data_free(ctx_data);

Py_XDECREF(loop);
Py_XDECREF(asyncio);

return NXT_UNIT_ERROR;
}

#else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can also be compacted to a single one with small preprocessor in it.

@ac000
Copy link
Member

ac000 commented Feb 7, 2023

This has been fixed via 4b7a954, 9c0a4a0, cbc0190

@ac000 ac000 closed this Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants