From 243cb10e2ab679623503b95b37418e56dfe01244 Mon Sep 17 00:00:00 2001 From: Maxime Poulin Date: Wed, 8 Feb 2023 22:05:22 -0500 Subject: [PATCH 1/5] Don't crash on oidentd socket race condition --- server/identification.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/identification.ts b/server/identification.ts index 12a08028..7362ef9a 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -118,7 +118,13 @@ class Identification { this.connections.forEach((connection) => { if (!connection.socket.remotePort || !connection.socket.localPort) { - throw new Error("Socket has no remote or local port"); + // Race condition: this can happen when more than one socket gets disconnected at + // once, since we `refresh()` for each one being added/removed. This results + // in there possibly be one or more disconnected sockets remaining when we get here. + // + // Simply skip this socket and not crash the server. + log.warn("oidentd: socket has no remote or local port?"); + return; } file += From 26b7fbf2c0a7ef9185b51948ae930d6e8a8a6a49 Mon Sep 17 00:00:00 2001 From: Max Leiter Date: Mon, 13 Feb 2023 20:50:16 -0800 Subject: [PATCH 2/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mina Galić --- server/identification.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/identification.ts b/server/identification.ts index 7362ef9a..48b88c59 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -120,9 +120,9 @@ class Identification { if (!connection.socket.remotePort || !connection.socket.localPort) { // Race condition: this can happen when more than one socket gets disconnected at // once, since we `refresh()` for each one being added/removed. This results - // in there possibly be one or more disconnected sockets remaining when we get here. + // in there possibly being one or more disconnected sockets remaining when we get here. // - // Simply skip this socket and not crash the server. + // Simply skip this socket and do not crash the server. log.warn("oidentd: socket has no remote or local port?"); return; } From 4cff2ccabe76ed42ae4c569fe24ce0538727e568 Mon Sep 17 00:00:00 2001 From: Max Leiter Date: Mon, 13 Feb 2023 20:51:27 -0800 Subject: [PATCH 3/5] Link to PR in log.warn --- server/identification.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/identification.ts b/server/identification.ts index 48b88c59..23912ef7 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -123,7 +123,7 @@ class Identification { // in there possibly being one or more disconnected sockets remaining when we get here. // // Simply skip this socket and do not crash the server. - log.warn("oidentd: socket has no remote or local port?"); + log.warn("oidentd: socket has no remote or local port. See https://github.com/thelounge/thelounge/pull/4695."); return; } From e9a09f54474dc4448931da76fc47a1efa5a7b686 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 18 Feb 2023 11:34:28 +0100 Subject: [PATCH 4/5] Add id to error log --- server/identification.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/server/identification.ts b/server/identification.ts index 23912ef7..6d7af382 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -116,14 +116,12 @@ class Identification { refresh() { let file = "# Warning: file generated by The Lounge: changes will be overwritten!\n"; - this.connections.forEach((connection) => { + this.connections.forEach((connection, id) => { if (!connection.socket.remotePort || !connection.socket.localPort) { // Race condition: this can happen when more than one socket gets disconnected at // once, since we `refresh()` for each one being added/removed. This results // in there possibly being one or more disconnected sockets remaining when we get here. - // - // Simply skip this socket and do not crash the server. - log.warn("oidentd: socket has no remote or local port. See https://github.com/thelounge/thelounge/pull/4695."); + log.warn(`oidentd: socket has no remote or local port (id=${id}). See https://github.com/thelounge/thelounge/pull/4695.`); return; } From bdc1f231079489acffc498f21d9fc65cd63d9913 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 18 Feb 2023 11:46:31 +0100 Subject: [PATCH 5/5] fix formatting --- server/identification.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/identification.ts b/server/identification.ts index 6d7af382..1c01d793 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -121,7 +121,9 @@ class Identification { // Race condition: this can happen when more than one socket gets disconnected at // once, since we `refresh()` for each one being added/removed. This results // in there possibly being one or more disconnected sockets remaining when we get here. - log.warn(`oidentd: socket has no remote or local port (id=${id}). See https://github.com/thelounge/thelounge/pull/4695.`); + log.warn( + `oidentd: socket has no remote or local port (id=${id}). See https://github.com/thelounge/thelounge/pull/4695.` + ); return; }