squeaky-pl / japronto

Screaming-fast Python 3.5+ HTTP toolkit integrated with pipelining HTTP server based on uvloop and picohttpparser.
MIT License
8.61k stars 581 forks source link

connection_made errors out with latest uvloop. #122

Closed btegs closed 6 years ago

btegs commented 6 years ago

I tried a very basic tutorial at https://github.com/squeaky-pl/japronto/blob/master/tutorial/1_hello.md with pip 10.0.1, japronto 0.1.1, uvloop 0.9.1, and ujson 1.35 on Ubuntu 18.04 64-bit running Python 3.6.5. I have tried the latest Sanic with this uvloop and it doesn't cause any errors with it.

When I start the server, it instantly shows this in the terminal:
Accepting connections on http://0.0.0.0:8080

but when I go to that in my browser, the browser is stuck in a loading animation and it immediately errors out in the terminal with this output:

Exception in callback UVTransport._call_connection_made
handle: <Handle UVTransport._call_connection_made>
Traceback (most recent call last):
  File "uvloop/cbhandles.pyx", line 52, in uvloop.loop.Handle._run
  File "uvloop/handles/tcp.pyx", line 149, in uvloop.loop.TCPTransport._call_connection_made
  File "uvloop/handles/basetransport.pyx", line 156, in uvloop.loop.UVBaseTransport._call_connection_made
  File "uvloop/handles/basetransport.pyx", line 153, in uvloop.loop.UVBaseTransport._call_connection_made
SystemError: NULL result without error in PyObject_Call
Exception in callback UVTransport._call_connection_made
handle: <Handle UVTransport._call_connection_made>
Traceback (most recent call last):
  File "uvloop/cbhandles.pyx", line 52, in uvloop.loop.Handle._run
  File "uvloop/handles/tcp.pyx", line 149, in uvloop.loop.TCPTransport._call_connection_made
  File "uvloop/handles/basetransport.pyx", line 156, in uvloop.loop.UVBaseTransport._call_connection_made
  File "uvloop/handles/basetransport.pyx", line 153, in uvloop.loop.UVBaseTransport._call_connection_made
SystemError: NULL result without error in PyObject_Call

I know this project hasn't been maintained in a while, but a bunch of benchmarking sites like https://github.com/tbrand/which_is_the_fastest are starting to list it as the best Python server competing with Rust, Go, and Node.

Will there be updates to this in the near future and if so, will there be more testing?

squeaky-pl commented 6 years ago

I don't have time to look into it but i can give you hints to fix it / or somebody else that knows C can fix it easily.

In your traceback you can see that connection_made returned NULL which is not valid return type without settings exception.

Now the source code for connection_made sits here: https://github.com/squeaky-pl/japronto/blob/master/src/japronto/protocol/cprotocol.c#L182

static PyObject*
Protocol_connection_made(Protocol* self, PyObject* transport)
{
#ifdef PROTOCOL_TRACK_REFCNT
  printf("made: %ld, %ld, %ld, ",
    (size_t)Py_REFCNT(Py_None), (size_t)Py_REFCNT(Py_True), (size_t)Py_REFCNT(Py_False));
  self->none_cnt = Py_REFCNT(Py_None);
  self->true_cnt = Py_REFCNT(Py_True);
  self->false_cnt = Py_REFCNT(Py_False);
#endif

  PyObject* get_extra_info = NULL;
  PySocketSockObject* socket = NULL;
  PyObject* connections = NULL;
  self->transport = transport;
  Py_INCREF(self->transport);

  if(!(get_extra_info = PyObject_GetAttrString(transport, "get_extra_info")))
    goto error;

  if(!(socket = (PySocketSockObject*)PyObject_CallFunctionObjArgs(
       get_extra_info, socket_str, NULL)))
    goto error;

  const int on = 1;

  if(setsockopt(socket->sock_fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) != 0)
    goto error;

  if(!(self->write = PyObject_GetAttrString(transport, "write")))
    goto error;

  if(!(self->writelines = PyObject_GetAttrString(transport, "writelines")))
    goto error;

  if(!(connections = PyObject_GetAttrString(self->app, "_connections")))
    goto error;

#ifdef REAPER_ENABLED
  self->idle_time = 0;
  self->read_ops = 0;
  self->last_read_ops = 0;
#endif

  if(PySet_Add(connections, (PyObject*)self) == -1)
    goto error;

  self->closed = false;

  goto finally;

  error:
  return NULL;

  finally:
  Py_XDECREF(connections);
  Py_XDECREF(socket);
  Py_XDECREF(get_extra_info);
  Py_RETURN_NONE;
}

you can see that there are various error conditions that lead to returning NULL (goto error), you would need to check with one fails on newer uvloop and provide a fix.

squeaky-pl commented 6 years ago

A wild guess:

if(setsockopt(socket->sock_fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) != 0)
    goto error;

fails.

btegs commented 6 years ago

This isn't my project, so the fixing will have to come on yours or uvloop's end.

squeaky-pl commented 6 years ago

Concerning activity, I added the following warning to the README:

There is no new project development happening at the moment, but it's not abandoned either.

If you are a novice Python programmer, you don't like plumbing yourself or you don't have basic understading of C, this project is not probably what you are looking for.
btegs commented 6 years ago

That addition you made to the README comes off as a passive aggressive attack, but it is what it is. There is no fault in wanting to use an existing framework and prioritize our personal and/or company time on developing applications on top of a stable base instead of having to constantly tweak the plumbing behind someone else's abandoned work. I am probably not alone in hearing about this project via word of mouth and wanting to see how it compared against Django, Twisted, Falcon, and Sanic to name a few. After seeing the direction where this project is going due to your attitude and lack of commitment, it is best at this point to go with a more focused project like Sanic.

syrusakbary commented 6 years ago

I just read this thread (as I had the same issue), and sincerely I'm a bit concerned from the comments on it. By commenting here I don't pretend to fan the flames, but to add a point of view that might help any user of an open-source project.

@btegs I'm an open-source developer (with some projects with a relative success and usage across different companies). Almost all the time I dedicated to it came from my own personal time (not paid by anyone).

I think is quite important that we take a proactive approach when creating an issue. It seems that @squeaky-pl already commented that he don't have that much time to look into it, so here are some ways that could have helped to address this issue more proactively:

Forcing someone to spend their free time on fixing something that you need is probably not the best path, since it creates much more rejection from the contributors than willingness for fixing it.

Hope this comment helps and keep up the good work!

btegs commented 6 years ago

@syrusakbary - Thanks for chiming in on the situation. I really really really want this project to succeed as it gives Python, a language I love, a fast framework to compete with Node, Rust, and Go for web frameworks. It is disheartening to see the creator of the project pretty much give up on it in favor of using japronto for non-free commercial projects where the fixes he implements for his clients don't make it back to the main project.

As for contributing, I don't know C on the same level as the others here or the main project lead. It would be great if @squeaky-pl quickly fixed the issue so breakages stop in future versions of uvloop. Squeaky has put together a great project and if he knows exactly how to fix his own code, he should show some effort in fixing it for everyone else. Just take an hour or so of your day to make it work.

squeaky-pl commented 6 years ago

@btegs even though it's probably a trivial fix you are underestimating the cost of jumping back to the context, going through the whole release process, waiting for OSX builds on travis (can easily take hours) and fixing what else might appear in the process.

Easy fix doesnt mean no time involved.

Commercial work pays my bills, open source does not.

btegs commented 6 years ago

"Commercial work pays my bills, open source does not."

You might as well just shut down this project at this point with that attitude as it is a big "f*ck you" to people that gave this project a chance.

waghanza commented 6 years ago

Hello everyone,

We use japronto on https://github.com/tbrand/which_is_the_fastest as a competition candidate.

As the main maintainer of this project, I totally understand @squeaky-pl that being an open source developer / maintainer take times and then the decision of keep working / maintaining any tool IS personal / COULD not be disputed.

However, I find this project awesome and will be happy to see that anyone keep maintaining japronto.

We can not blame @squeaky-pl for his decision, the only thing we can do is ensuring a transition, either by maintaining or calling for maintenance (in README, on https://gitter.im/japronto/Lobby ...).

:heart:

ifplusor commented 6 years ago

@squeaky-pl you guess is right, this line return 9 errno. in my test, get_extra_info have some question.

ifplusor commented 6 years ago

after read uvloop code, i found the reason. since uvloop 0.9.0, call TCPTransport.get_extra_info('socket') will return a PseudoSocket(define in python) object instead of socket_socket. and its layer is different than PySocketSockObject, except they (possible) have same interface, of cause fileno().

squeaky-pl commented 6 years ago

The change was released to PYPI as version 0.1.2. Thank you.

waghanza commented 6 years ago

Awesome, this issue was fixed for me with the last uvloop, 0.11.3

:tada: I've update japronto in following benchmark https://github.com/the-benchmarker/web-frameworks/pull/607

Results will be updated soon :heart: