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

Adapt bind_socket to make it usable with multiple processes #1009

Merged
merged 14 commits into from Jun 21, 2021
23 changes: 23 additions & 0 deletions tests/test_config.py
Expand Up @@ -2,6 +2,7 @@
import logging
import os
import socket
import sys
from copy import deepcopy

import pytest
Expand Down Expand Up @@ -320,3 +321,25 @@ def test_ws_max_size():
config = Config(app=asgi_app, ws_max_size=1000)
config.load()
assert config.ws_max_size == 1000


@pytest.mark.parametrize(
"reload, workers",
[
(True, 1),
(False, 2),
],
ids=["--reload=True --workers=1", "--reload=False --workers=2"],
)
@pytest.mark.skipif(sys.platform == "win32", reason="require unix-like system")
def test_bind_unix_socket_works_with_reload_or_workers(tmp_path, reload, workers):
uds_file = tmp_path / "uvicorn.sock"
config = Config(
app=asgi_app, uds=uds_file.as_posix(), reload=reload, workers=workers
Kludex marked this conversation as resolved.
Show resolved Hide resolved
)
config.load()
sock = config.bind_socket()
assert isinstance(sock, socket.socket)
assert sock.family == socket.AF_UNIX
assert sock.getsockname() == uds_file.as_posix()
sock.close()
94 changes: 66 additions & 28 deletions uvicorn/config.py
Expand Up @@ -366,37 +366,75 @@ def setup_event_loop(self):
loop_setup()

def bind_socket(self):
family = socket.AF_INET
addr_format = "%s://%s:%d"
if self.uds:
path = self.uds
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
try:
sock.bind(path)
uds_perms = 0o666
os.chmod(self.uds, uds_perms)
except OSError as exc:
logger.error(exc)
sys.exit(1)

if self.host and ":" in self.host:
# It's an IPv6 address.
family = socket.AF_INET6
addr_format = "%s://[%s]:%d"
message = "Uvicorn running on unix socket %s (Press CTRL+C to quit)"
sock_name_format = "%s"
color_message = (
"Uvicorn running on "
+ click.style(sock_name_format, bold=True)
+ " (Press CTRL+C to quit)"
)
logger.info(
message,
self.uds,
extra={"color_message": color_message},
)
elif self.fd:
sock = socket.fromfd(self.fd, socket.AF_UNIX, socket.SOCK_STREAM)
message = "Uvicorn running on socket %s (Press CTRL+C to quit)"
fd_name_format = "%s"
color_message = (
"Uvicorn running on "
+ click.style(fd_name_format, bold=True)
+ " (Press CTRL+C to quit)"
)
logger.info(
message,
sock.getsockname(),
extra={"color_message": color_message},
)
else:
family = socket.AF_INET
addr_format = "%s://%s:%d"

if self.host and ":" in self.host:
# It's an IPv6 address.
family = socket.AF_INET6
addr_format = "%s://[%s]:%d"

sock = socket.socket(family=family)
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
try:
sock.bind((self.host, self.port))
except OSError as exc:
logger.error(exc)
sys.exit(1)

sock = socket.socket(family=family)
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
try:
sock.bind((self.host, self.port))
except OSError as exc:
logger.error(exc)
sys.exit(1)
message = f"Uvicorn running on {addr_format} (Press CTRL+C to quit)"
color_message = (
"Uvicorn running on "
+ click.style(addr_format, bold=True)
+ " (Press CTRL+C to quit)"
)
protocol_name = "https" if self.is_ssl else "http"
logger.info(
Kludex marked this conversation as resolved.
Show resolved Hide resolved
message,
protocol_name,
self.host,
self.port,
extra={"color_message": color_message},
)
sock.set_inheritable(True)
Kludex marked this conversation as resolved.
Show resolved Hide resolved

message = f"Uvicorn running on {addr_format} (Press CTRL+C to quit)"
color_message = (
"Uvicorn running on "
+ click.style(addr_format, bold=True)
+ " (Press CTRL+C to quit)"
)
protocol_name = "https" if self.is_ssl else "http"
logger.info(
message,
protocol_name,
self.host,
self.port,
extra={"color_message": color_message},
)
return sock

@property
Expand Down
3 changes: 3 additions & 0 deletions uvicorn/main.py
@@ -1,4 +1,5 @@
import logging
import os
import platform
import ssl
import sys
Expand Down Expand Up @@ -391,6 +392,8 @@ def run(app: typing.Union[ASGIApplication, str], **kwargs: typing.Any) -> None:
Multiprocess(config, target=server.run, sockets=[sock]).run()
else:
server.run()
if config.uds:
os.remove(config.uds)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Maybe I'm mistaken, but can't we have a single one of those outside the conditional?

Suggested change
if config.uds:
os.remove(config.uds)
if config.uds:
os.remove(config.uds)

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I guess we can, but it's not right, because we bind inside them. 🤔 Right?

Copy link
Member Author

Choose a reason for hiding this comment

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

it looks simpler yes, I removed occurrences in the basereload and multiprocess shutdown method and put it outside



if __name__ == "__main__":
Expand Down
2 changes: 2 additions & 0 deletions uvicorn/supervisors/basereload.py
Expand Up @@ -76,6 +76,8 @@ def restart(self) -> None:

def shutdown(self) -> None:
self.process.join()
if self.config.uds:
os.remove(self.config.uds)
message = "Stopping reloader process [{}]".format(str(self.pid))
color_message = "Stopping reloader process [{}]".format(
click.style(str(self.pid), fg="cyan", bold=True)
Expand Down
3 changes: 2 additions & 1 deletion uvicorn/supervisors/multiprocess.py
Expand Up @@ -65,7 +65,8 @@ def startup(self) -> None:
def shutdown(self) -> None:
for process in self.processes:
process.join()

if self.config.uds:
os.remove(self.config.uds)
message = "Stopping parent process [{}]".format(str(self.pid))
color_message = "Stopping parent process [{}]".format(
click.style(str(self.pid), fg="cyan", bold=True)
Expand Down