mirror of
https://github.com/certbot/certbot.git
synced 2026-08-03 00:22:04 +02:00
Merge pull request #419 from letsencrypt/fix_standalone_signal_race
Attempt to fix #378
This commit is contained in:
@@ -152,9 +152,6 @@ class StandaloneAuthenticator(common.Plugin):
|
|||||||
:rtype: bool
|
:rtype: bool
|
||||||
|
|
||||||
"""
|
"""
|
||||||
signal.signal(signal.SIGIO, self.client_signal_handler)
|
|
||||||
signal.signal(signal.SIGUSR1, self.client_signal_handler)
|
|
||||||
signal.signal(signal.SIGUSR2, self.client_signal_handler)
|
|
||||||
|
|
||||||
display = zope.component.getUtility(interfaces.IDisplay)
|
display = zope.component.getUtility(interfaces.IDisplay)
|
||||||
|
|
||||||
@@ -259,6 +256,16 @@ class StandaloneAuthenticator(common.Plugin):
|
|||||||
:rtype: bool
|
:rtype: bool
|
||||||
|
|
||||||
"""
|
"""
|
||||||
|
# In order to avoid a race condition, we set the signal handler
|
||||||
|
# that will be needed by the parent process now, and undo this
|
||||||
|
# action if we turn out to be the child process. (This needs
|
||||||
|
# to happen before the fork because the child will send one of
|
||||||
|
# these signals to the parent almost immediately after the
|
||||||
|
# fork, and the parent must already be ready to receive it.)
|
||||||
|
signal.signal(signal.SIGIO, self.client_signal_handler)
|
||||||
|
signal.signal(signal.SIGUSR1, self.client_signal_handler)
|
||||||
|
signal.signal(signal.SIGUSR2, self.client_signal_handler)
|
||||||
|
|
||||||
fork_result = os.fork()
|
fork_result = os.fork()
|
||||||
Crypto.Random.atfork()
|
Crypto.Random.atfork()
|
||||||
if fork_result:
|
if fork_result:
|
||||||
@@ -269,6 +276,12 @@ class StandaloneAuthenticator(common.Plugin):
|
|||||||
return self.do_parent_process(port)
|
return self.do_parent_process(port)
|
||||||
else:
|
else:
|
||||||
# CHILD process (the TCP listener subprocess)
|
# CHILD process (the TCP listener subprocess)
|
||||||
|
# Undo the parent's signal handler settings, which aren't
|
||||||
|
# applicable to us.
|
||||||
|
signal.signal(signal.SIGIO, signal.SIG_DFL)
|
||||||
|
signal.signal(signal.SIGUSR1, signal.SIG_DFL)
|
||||||
|
signal.signal(signal.SIGUSR2, signal.SIG_DFL)
|
||||||
|
|
||||||
self.child_pid = os.getpid()
|
self.child_pid = os.getpid()
|
||||||
# do_child_process() is normally not expected to return but
|
# do_child_process() is normally not expected to return but
|
||||||
# should terminate via sys.exit().
|
# should terminate via sys.exit().
|
||||||
|
|||||||
@@ -404,47 +404,39 @@ class DoParentProcessTest(unittest.TestCase):
|
|||||||
StandaloneAuthenticator
|
StandaloneAuthenticator
|
||||||
self.authenticator = StandaloneAuthenticator(config=None, name=None)
|
self.authenticator = StandaloneAuthenticator(config=None, name=None)
|
||||||
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal")
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
||||||
"zope.component.getUtility")
|
"zope.component.getUtility")
|
||||||
def test_do_parent_process_ok(self, mock_get_utility, mock_signal):
|
def test_do_parent_process_ok(self, mock_get_utility):
|
||||||
self.authenticator.subproc_state = "ready"
|
self.authenticator.subproc_state = "ready"
|
||||||
result = self.authenticator.do_parent_process(1717)
|
result = self.authenticator.do_parent_process(1717)
|
||||||
self.assertTrue(result)
|
self.assertTrue(result)
|
||||||
self.assertEqual(mock_get_utility.call_count, 1)
|
self.assertEqual(mock_get_utility.call_count, 1)
|
||||||
self.assertEqual(mock_signal.call_count, 3)
|
|
||||||
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal")
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
||||||
"zope.component.getUtility")
|
"zope.component.getUtility")
|
||||||
def test_do_parent_process_inuse(self, mock_get_utility, mock_signal):
|
def test_do_parent_process_inuse(self, mock_get_utility):
|
||||||
self.authenticator.subproc_state = "inuse"
|
self.authenticator.subproc_state = "inuse"
|
||||||
result = self.authenticator.do_parent_process(1717)
|
result = self.authenticator.do_parent_process(1717)
|
||||||
self.assertFalse(result)
|
self.assertFalse(result)
|
||||||
self.assertEqual(mock_get_utility.call_count, 1)
|
self.assertEqual(mock_get_utility.call_count, 1)
|
||||||
self.assertEqual(mock_signal.call_count, 3)
|
|
||||||
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal")
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
||||||
"zope.component.getUtility")
|
"zope.component.getUtility")
|
||||||
def test_do_parent_process_cantbind(self, mock_get_utility, mock_signal):
|
def test_do_parent_process_cantbind(self, mock_get_utility):
|
||||||
self.authenticator.subproc_state = "cantbind"
|
self.authenticator.subproc_state = "cantbind"
|
||||||
result = self.authenticator.do_parent_process(1717)
|
result = self.authenticator.do_parent_process(1717)
|
||||||
self.assertFalse(result)
|
self.assertFalse(result)
|
||||||
self.assertEqual(mock_get_utility.call_count, 1)
|
self.assertEqual(mock_get_utility.call_count, 1)
|
||||||
self.assertEqual(mock_signal.call_count, 3)
|
|
||||||
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal")
|
|
||||||
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
@mock.patch("letsencrypt.plugins.standalone.authenticator."
|
||||||
"zope.component.getUtility")
|
"zope.component.getUtility")
|
||||||
def test_do_parent_process_timeout(self, mock_get_utility, mock_signal):
|
def test_do_parent_process_timeout(self, mock_get_utility):
|
||||||
# Normally times out in 5 seconds and returns False. We can
|
# Normally times out in 5 seconds and returns False. We can
|
||||||
# now set delay_amount to a lower value so that it times out
|
# now set delay_amount to a lower value so that it times out
|
||||||
# faster than it would under normal use.
|
# faster than it would under normal use.
|
||||||
result = self.authenticator.do_parent_process(1717, delay_amount=1)
|
result = self.authenticator.do_parent_process(1717, delay_amount=1)
|
||||||
self.assertFalse(result)
|
self.assertFalse(result)
|
||||||
self.assertEqual(mock_get_utility.call_count, 1)
|
self.assertEqual(mock_get_utility.call_count, 1)
|
||||||
self.assertEqual(mock_signal.call_count, 3)
|
|
||||||
|
|
||||||
|
|
||||||
class DoChildProcessTest(unittest.TestCase):
|
class DoChildProcessTest(unittest.TestCase):
|
||||||
|
|||||||
Reference in New Issue
Block a user