Skip to content
This repository was archived by the owner on Nov 23, 2017. It is now read-only.

#425 Remove UNIX socket from FS before binding. #441

Merged
merged 1 commit into from
Oct 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions asyncio/unix_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,17 @@ def create_unix_server(self, protocol_factory, path=None, *,

sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)

# Check for abstract socket. `str` and `bytes` paths are supported.
if path[0] not in (0, '\x00'):
try:
if stat.S_ISSOCK(os.stat(path).st_mode):
os.remove(path)

Choose a reason for hiding this comment

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

What if the file exists and is not a socket?

Copy link
Author

@socketpair socketpair Oct 6, 2016

Choose a reason for hiding this comment

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

if stat.S_ISSOCK(os.stat(path).st_mode)

As much as we can. Let's kernel to decide what to do during bind()

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand: can stat.S_ISSOCK return True if it's not a socket?

Copy link
Author

Choose a reason for hiding this comment

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

No, it can not. If this path refers to plain file, bind syscall will fail. We must not erase non-sockets in any case.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, OK, I now understand what was @ZhukovAlexander's question about.

Copy link
Author

Choose a reason for hiding this comment

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

Do you still have a questions ?

Copy link
Member

@1st1 1st1 Oct 7, 2016

Choose a reason for hiding this comment

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

No, no further questions about this line :)

except FileNotFoundError:
pass
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to log this.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, but we should introduce something like log = logging.getLogger(__name__). Please tell where to do that.

Copy link
Member

Choose a reason for hiding this comment

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

Look at how base_events.py logs stuff.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

except OSError as err:
# Directory may have permissions only to create socket.
logger.error('Unable to check or remove stale UNIX socket %r: %r', path, err)

try:
sock.bind(path)
except OSError as exc:
Expand Down
10 changes: 5 additions & 5 deletions tests/test_unix_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,11 @@ def test_create_unix_server_existing_path_sock(self):
with test_utils.unix_socket_path() as path:
sock = socket.socket(socket.AF_UNIX)
sock.bind(path)
with sock:
coro = self.loop.create_unix_server(lambda: None, path)
with self.assertRaisesRegex(OSError,
'Address.*is already in use'):
self.loop.run_until_complete(coro)
sock.listen(1)
sock.close()

coro = self.loop.create_unix_server(lambda: None, path)
self.loop.run_until_complete(coro)

def test_create_unix_server_existing_path_nonsock(self):
with tempfile.NamedTemporaryFile() as file:
Expand Down