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

Importing json stops additional asyncio servers from running #329

Closed
atbentley opened this issue Jan 21, 2017 · 11 comments
Closed

Importing json stops additional asyncio servers from running #329

atbentley opened this issue Jan 21, 2017 · 11 comments

Comments

@atbentley
Copy link
Contributor

atbentley commented Jan 21, 2017

This is related to running additional asyncio servers alongside sanic on the same event loop.

OS: macOS 10.12.2
Python: 3.6.0
Sanic: 0.2.0
uvloop: 0.7.2

Here is a working example that shows two servers running on the same event loop, a simple Sanic http server with a single endpoint at / that returns "ok" and a TCP echo server.

import asyncio
import uvloop
from sanic import Sanic
from sanic.response import HTTPResponse


class EchoProtocol(asyncio.Protocol):
    def connection_made(self, transport):
        self.transport = transport

    def data_received(self, data):
        self.transport.write(data)


asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
loop = asyncio.get_event_loop()
loop.run_until_complete(loop.create_server(EchoProtocol, '127.0.0.1', 8888))

app = Sanic()
@app.route('/')
async def index(request):
    return HTTPResponse(status=200, body='ok')
app.run(host='127.0.0.1', port=8889, loop=loop)

Run the python script and verify it works by checking the echo server and http servers on ports 8888 and 8889 respectively:

$ echo test | nc 127.0.0.1 8888
test
$ curl 127.0.0.1:8889
ok

To break it, throw in a import json up with the other imports, run the script then hit the servers:

$ echo test | nc 127.0.0.1 8888
$ curl 127.0.0.1:8889
ok

The echo server has stopped responding, checking the os to see which ports are bound (on my mac netstat -anp tcp | grep 8888) shows that the echo server never bound its port, but the http server did bind its port.

To further complicate matters, commenting out the line that sets the event loop policy to use uvloop makes everything work again.

# asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
$ echo test | nc 127.0.0.1 8888
test
$ curl 127.0.0.1:8889
ok

However I have raised this as an issue on Sanic because if we remove the Sanic server and replace it with a second echo server both of them work:

import asyncio
import uvloop
import json

class EchoProtocol(asyncio.Protocol):
    def connection_made(self, transport):
        self.transport = transport

    def data_received(self, data):
        self.transport.write(data)


asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
loop = asyncio.get_event_loop()
loop.run_until_complete(loop.create_server(EchoProtocol, '127.0.0.1', 8888))
loop.run_until_complete(loop.create_server(EchoProtocol, '127.0.0.1', 8889))
loop.run_forever()

Then probe the servers:

$ echo test | nc 127.0.0.1 8888
test
$ echo test again | nc 127.0.0.1 8889
test again

Which makes me believe this is an issue with Sanic, however the root issue may be related to uvloop.

@r0fls
Copy link
Contributor

r0fls commented Jan 22, 2017

I can't reproduce this. Importing json causes no issues with Sanic for me with your example. However the EchoProtocol you have is not working for me, either way. It just hangs and doesn't respond.

@atbentley
Copy link
Contributor Author

Sorry, maybe I didn't phrase it quite right. Without the json import both the sanic server and the echo server run, with the json import the sanic server runs but the echo server fails to bind. What you're describing sounds like a reproduction, can you check again and confirm?

I say 'fails to bind' rather than hangs because I can't see the 8888 port bound on my computer, running a telnet to it gives me the following:

$ telnet 127.0.0.1 8888
Trying 127.0.0.1...
telnet: connect to address 127.0.0.1: Connection refused
telnet: Unable to connect to remote host

I've done a bit more digging and I was able to successfully reproduce on a second mac machine:

OS: OS X 10.11.6
Python: 3.6.0
Sanic: 0.2.0
uvloop: 0.7.2

Tested on ubuntu 16.04 with Python 3.6.0 and was not able to reproduce.

Switching to python 3.5.0 on both macs fixes the issue.

So perhaps this may be an issue with python 3.6.0 on mac?

@r0fls
Copy link
Contributor

r0fls commented Jan 22, 2017

@atbentley can you try doing this without passing in loop=loop to run? I don't think that's correct, and I think we'll be deprecating that eventually and no longer supporting it. Yury advises against it here: #152 (comment)

While I hate to call to authority, he's the author of uvloop and the await/async PEP (https://www.python.org/dev/peps/pep-0492/). Furthermore, passing the loop is related to approximately 5 open issues we have. So, we're probably going to have to find a way to avoid doing that in general.

@atbentley
Copy link
Contributor Author

I'm not really sold on the passing loop=loop in anyway, so thats alright. However I don't believe there is a way to run a second asyncio server with sanic 0.2 without passing in loop=loop. When calling app.run sanic will create a new loop, set it as the current loop, register it's own server on that loop then call run_forever which doesn't give us any opportunity to register a server.

That said, I threw in a return statement just before run_forever in server.py and did the echo server initialisation post the sanic initialisation and things seem to be working .

import asyncio
import uvloop
from sanic import Sanic
from sanic.response import HTTPResponse
import json

class EchoProtocol(asyncio.Protocol):
    def connection_made(self, transport):
        self.transport = transport

    def data_received(self, data):
        self.transport.write(data)

app = Sanic()
@app.route('/')
async def index(request):
    return HTTPResponse(status=200, body='ok')
app.run(host='127.0.0.1', port=8889)
loop = asyncio.get_event_loop()
loop.run_until_complete(loop.create_server(EchoProtocol, '127.0.0.1', 8888))
loop.run_forever()
$ curl 127.0.0.1:8889
ok%
$ echo test | nc 127.0.0.1 8888
test

I figure that the resolutions to #152 and #275 may inadvertently solve this, however I would take care to see if there is a difference between initialising an extra server before sanic has been initialised as opposed to after sanic has been initialised.

@r0fls
Copy link
Contributor

r0fls commented Jan 22, 2017

@atbentley why do you need a second server? You should be using multiple workers app.run(workers=4)

@r0fls
Copy link
Contributor

r0fls commented Jan 22, 2017

Also, you can run separate servers on separate ports:

app1.py

from sanic import Sanic
from sanic.response import json

app = Sanic(__name__)


@app.route("/")
async def test(request):
    return json({"test": True})


app.run(host="0.0.0.0", port=8000)

app2.py

from sanic import Sanic
from sanic.response import json

app = Sanic(__name__)


@app.route("/")
async def test(request):
    return json({"test": True})


app.run(host="0.0.0.0", port=8001)

running both of these at the same time works ^^

@atbentley
Copy link
Contributor Author

I'm running a websockets server on the same loop. Currently I have an API endpoint which is pushing data onto a queue and the messages on that queue get pushed down a websocket to some clients. What's the best practice in this regard, was sanic written with the intention of being the only server on a loop?

@r0fls
Copy link
Contributor

r0fls commented Jan 22, 2017

Ok, that makes sense. Hopefully eventually we'll have native websocket support. There's an open issue tracking it here: #12.

The websocket server and sanic both respond in this example for me:

import asyncio
import websockets
from sanic import Sanic
from sanic.response import text


async def hello(websocket, path):
    name = await websocket.recv()
    print("< {}".format(name))

    greeting = "Hello {}!".format(name)
    await websocket.send(greeting)
    print("> {}".format(greeting))

start_server = websockets.serve(hello, 'localhost', 8001)

app = eSanic()
@app.route('/')
def hello(request):
    return text("OK")

app.run(port=8000, before_start=lambda app, loop: asyncio.get_event_loop().run_until_complete(start_server))

Are you able to integrate them in this way?

@atbentley
Copy link
Contributor Author

Ah of course, it never occurred to me to use before_start. This is now working, even with import json. Thanks for spending the time to try and find work arounds!

Will this be the pattern for adding additional servers onto the loop in the future? If it is I suppose we could close this now. Otherwise we can leave it open and I can come back when "loop sharing" is implemented and check if this is still an issue.

@r0fls
Copy link
Contributor

r0fls commented Jan 22, 2017

No, I think we'll eventually have better support for this. Like you said earlier, it looks like this would provide one potential solution: #275

Then you could do something like this (obviously untested):

server = app.create_server(app_run_kwargs)
other_server = loop.create_server(MyProtocol, 'host', '8000')
loop.run_until_complete(aynscio.gather(*[server, other_server]))
loop.run_forever()

@seemethere
Copy link
Member

Passing the loop to run was deprecated in 0.3.0.

Closing.

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

No branches or pull requests

3 participants