This is an archive of the discontinued Mercurial Phabricator instance.

Differential D919

zeroconf: do not crash if socket being read is closed by another thread
ClosedPublic

Authored by quark on Oct 2 2017, 11:23 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG5385b76fd1fd: zeroconf: do not crash if socket being read is closed by another thread
Summary

In zeroconf/__init__.py, there is:

server = Zeroconf.Zeroconf(ip)
l = listener()
Zeroconf.ServiceBrowser(server, "_hg._tcp.local.", l)
time.sleep(1)
server.close()

server.close() closes the underlying socket while the ServiceBrowser may
still have a background thread reading the socket. There could be a race
condition where the reading thread reads the closed socket, resulting in
EBADF crash. This patch catches the exception.

This makes test-paths.t pass with chg.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

quark created this revision.Oct 2 2017, 11:23 PM
Closed by commit rHG5385b76fd1fd: zeroconf: do not crash if socket being read is closed by another thread (authored by quark). · Explain WhyOct 4 2017, 9:16 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/zeroconf/Zeroconf.py (12 lines)

Diff 2402

hgext/zeroconf/Zeroconf.py

added __ne__ adjuncts where required added __ne__ adjuncts where required
ensure names end in '.local.' ensure names end in '.local.'
timeout on receiving socket for clean shutdown""" timeout on receiving socket for clean shutdown"""
__author__ = "Paul Scott-Murphy" __author__ = "Paul Scott-Murphy"
__email__ = "paul at scott dash murphy dot com" __email__ = "paul at scott dash murphy dot com"
__version__ = "0.12" __version__ = "0.12"
import errno
import itertools import itertools
import select import select
import socket import socket
import string import string
import struct import struct
import threading import threading
import time import time
import traceback import traceback
It requires registration with an Engine object in order to have It requires registration with an Engine object in order to have
the read() method called when a socket is available for reading.""" the read() method called when a socket is available for reading."""
def __init__(self, zeroconf): def __init__(self, zeroconf):
self.zeroconf = zeroconf self.zeroconf = zeroconf
self.zeroconf.engine.addReader(self, self.zeroconf.socket) self.zeroconf.engine.addReader(self, self.zeroconf.socket)
def handle_read(self): def handle_read(self):
data, (addr, port) = self.zeroconf.socket.recvfrom(_MAX_MSG_ABSOLUTE) data = addr = port = None
sock = self.zeroconf.socket
try:
data, (addr, port) = sock.recvfrom(_MAX_MSG_ABSOLUTE)
except socket.error as e:
if e.errno == errno.EBADF:
# some other thread may close the socket
return
else:
raise
self.data = data self.data = data
msg = DNSIncoming(data) msg = DNSIncoming(data)
if msg.isQuery(): if msg.isQuery():
# Always multicast responses # Always multicast responses
# #
if port == _MDNS_PORT: if port == _MDNS_PORT:
self.zeroconf.handleQuery(msg, _MDNS_ADDR, _MDNS_PORT) self.zeroconf.handleQuery(msg, _MDNS_ADDR, _MDNS_PORT)
# If it's not a multicast query, reply via unicast # If it's not a multicast query, reply via unicast