FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-84532: Reopen the socket in SocketHandler.send() when sending fails by serhiy-storchaka · Pull Request #154540 · python/cpython · GitHub

Repository navigation

Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .py  (2) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
19 changes: 12 additions & 7 deletions Lib/logging/handlers.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -657,15 +657,20 @@ def send(self, s):
This function allows for partial sends which can happen when the
network is busy.
"""
if self.sock is None:
self.createSocket()
#self.sock can be None either because we haven't reached the retry
#time yet, or because we have reached the retry time and retried,
#but are still unable to connect.
if self.sock:
# Reopen the socket once if sending fails, e.g. the connection was
# closed after a timeout, so the record is not lost (gh-84532).
for attempt in range(2):
if self.sock is None:
self.createSocket()
# self.sock can be None either because we haven't reached the
# retry time yet, or because we have reached the retry time
# and retried, but are still unable to connect.
if self.sock is None:
return

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

How about a break here? I prefer not to do early returns - e.g. if we were ever to add something after the for statement, we wouldn't need to revisit this line.

try:
self.sock.sendall(s)
except OSError: #pragma: no cover
return

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

As comment above.

except OSError:
self.sock.close()
self.sock = None # so we can call createSocket next time

Expand Down
39 changes: 39 additions & 0 deletions Lib/test/test_logging.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import queue
import random
import re
import select
import shutil
import socket
import struct
Expand Down Expand Up @@ -1940,6 +1941,44 @@ def test_noserver(self):
time.sleep(self.sock_hdlr.retryTime - now + 0.001)
self.root_logger.error('Nor this')

def test_reconnect(self):
# If the server closes the connection, the record whose send fails is
# not dropped, but resent on a reopened socket (gh-84532).
server = socket.create_server(('localhost', 0))
self.addCleanup(server.close)
server.settimeout(support.LONG_TIMEOUT)
port = server.getsockname()[1]
hdlr = logging.handlers.SocketHandler('localhost', port)
self.addCleanup(hdlr.close)

def recv_message():
conn, _ = server.accept()
self.addCleanup(conn.close)
chunk = conn.recv(4)
slen = struct.unpack(">L", chunk)[0]
chunk = b''
while len(chunk) < slen:
chunk += conn.recv(slen - len(chunk))
return conn, logging.makeLogRecord(pickle.loads(chunk)).msg

# Connect and receive the first record.
hdlr.handle(logging.makeLogRecord({'msg': 'spam'}))
conn, msg = recv_message()
self.assertEqual(msg, 'spam')

# Close the connection on the server side. After a peer close the
# first send still succeeds (the data is lost when the reset arrives),
# so a throwaway send is used up here to make the next send fail.
conn.close()
# Wait until the close is received, so the sends below are ordered.
select.select([hdlr.sock], [], [], support.LONG_TIMEOUT)
hdlr.sock.sendall(b'\x00\x00\x00\x00')

# The next send fails and the record is resent on a new connection.
hdlr.handle(logging.makeLogRecord({'msg': 'eggs'}))
conn, msg = recv_message()
self.assertEqual(msg, 'eggs')


@unittest.skipUnless(hasattr(socket, "AF_UNIX"), "Unix sockets required")
class UnixSocketHandlerTest(SocketHandlerTest):
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:class:`logging.handlers.SocketHandler` now loses fewer log records when the
connection to the server is closed, for example after an idle timeout.
Loading

Back | FazBrowse Home | New Git URL