From 7934e622ca0c7b206d8a34246e30b5e1c692eb76 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 2 Jun 2026 15:51:19 -0400 Subject: [PATCH 1/7] =?UTF-8?q?=F0=9F=A7=B5=20Synchronize=20`@exception`?= =?UTF-8?q?=20ivar=20assignment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I'm not sure why this was outside the `synchronize`... --- lib/net/imap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 12b6c792..c648949c 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3568,8 +3568,8 @@ def receive_responses end end rescue Exception => e - @exception = e synchronize do + @exception = e @tagged_response_arrival.broadcast @continuation_request_arrival.broadcast end From 1275ecabc1daca65acf47adc3d54d8151d5cb320 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 2 Jun 2026 15:59:05 -0400 Subject: [PATCH 2/7] =?UTF-8?q?=F0=9F=A7=B5=20Move=20rescue=20clause=20ins?= =?UTF-8?q?ide=20synchronize=20block?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's no good reason to release the lock only to immediately reacquire it for exception handling. That might allow commands to be started (or continued) while still propagating irrelevant response errors. By moving the `rescue` clause inside the `synchronize` block, `begin` and `end` are now superfluous and can be removed. But that makes the diff seem much larger than it should, so I'm saving it for a later branch. --- lib/net/imap.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index c648949c..b184b786 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3566,9 +3566,7 @@ def receive_responses @response_handlers.each do |handler| handler.call(resp) end - end - rescue Exception => e - synchronize do + rescue Exception => e @exception = e @tagged_response_arrival.broadcast @continuation_request_arrival.broadcast From 0de8e4888f5a081665575a258be770cee2db2e74 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 4 Jun 2026 14:20:28 -0400 Subject: [PATCH 3/7] =?UTF-8?q?=F0=9F=A7=B5=20Shutdown=20receive=5Frespons?= =?UTF-8?q?es=20in=20ensure=20clause?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although it wasn't obvious at first glance, because nearly every possible error is caught by a `rescue Exception` clause, this is effectively no different from what was already happening. This structure makes that much more obvious. Also, by moving this code into the `ensure`, it allows us to simplify error handling elsewhere, by simply raising or re-raising an exception. This also lets us move `state_logout!` inside the `synchronize` block. --- lib/net/imap.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index b184b786..772fb5ca 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3573,6 +3573,7 @@ def receive_responses end end end + ensure synchronize do @receiver_thread_terminating = true @tagged_response_arrival.broadcast @@ -3580,9 +3581,8 @@ def receive_responses if @idle_done_cond @idle_done_cond.signal end + state_logout! end - ensure - state_logout! end def get_response From 589da11d2d9f411cc53523bae688563703a9d429 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 4 Jun 2026 18:36:30 -0400 Subject: [PATCH 4/7] =?UTF-8?q?=F0=9F=A7=B5=20Reset=20`@exception`=20after?= =?UTF-8?q?=20signaling=20condvars?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By resetting `@exception` inside the synchronize block at the end of the loop, we don't need to release and immediately reacquire the lock each time through the loop. --- lib/net/imap.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 772fb5ca..77c15070 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3517,9 +3517,6 @@ def tcp_socket(host, port) def receive_responses connection_closed = false until connection_closed - synchronize do - @exception = nil - end begin resp = get_response rescue Exception => e @@ -3570,6 +3567,8 @@ def receive_responses @exception = e @tagged_response_arrival.broadcast @continuation_request_arrival.broadcast + ensure + @exception = nil unless connection_closed end end end From a5f4bad6cff94843934b139c74d17bca6305753b Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 4 Jun 2026 14:20:28 -0400 Subject: [PATCH 5/7] =?UTF-8?q?=F0=9F=A7=B5=20Enter=20logout=20state=20bef?= =?UTF-8?q?ore=20signaling=20condvars?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The receiver thread now enters the logout state _before_ signaling the conditional variables for the last time. This is less surprising than doing it afterward, and it also makes some of the other `state_logout!` calls redundant. --- lib/net/imap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 77c15070..6333eebd 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3574,13 +3574,13 @@ def receive_responses end ensure synchronize do + state_logout! @receiver_thread_terminating = true @tagged_response_arrival.broadcast @continuation_request_arrival.broadcast if @idle_done_cond @idle_done_cond.signal end - state_logout! end end From a9b02f2468d1ddf9fe9aa685e7d516e34e5456ca Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 7 Jun 2026 15:53:30 -0400 Subject: [PATCH 6/7] =?UTF-8?q?=F0=9F=A7=B5=20Merge=20exception=20handling?= =?UTF-8?q?=20for=20get=5Fresponse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this change, an EOFError `@exception` may be assigned a receiver thread backtrace. Previously, it might've been raised in the client thread which, would set the backtrace for that client thread. --- lib/net/imap.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 6333eebd..01c75238 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3518,7 +3518,7 @@ def receive_responses connection_closed = false until connection_closed begin - resp = get_response + resp = get_response or raise EOFError, "end of file reached" rescue Exception => e synchronize do state_logout! @@ -3527,12 +3527,6 @@ def receive_responses end break end - unless resp - synchronize do - @exception = EOFError.new("end of file reached") - end - break - end begin synchronize do case resp From dcad6e69c5ef053ea4a7c26bcdb2f5c047f6e13b Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 7 Jun 2026 16:03:18 -0400 Subject: [PATCH 7/7] =?UTF-8?q?=F0=9F=A7=B5=20Set=20receive=5Fresponses=20?= =?UTF-8?q?`@exception`=20atomically?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This delays setting `@exception` until it can be synchronized with everything else in the `ensure` block. NOTE: this could be simplified further: drop the `rescue` clause and use `$!` in the `ensure` clause. But that seems to trigger a TruffleRuby bug: https://github.com/truffleruby/truffleruby/issues/4308 --- lib/net/imap.rb | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 01c75238..599dbcb3 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3515,18 +3515,10 @@ def tcp_socket(host, port) end def receive_responses + exception = nil connection_closed = false until connection_closed - begin - resp = get_response or raise EOFError, "end of file reached" - rescue Exception => e - synchronize do - state_logout! - @sock.close - @exception = e - end - break - end + resp = get_response or raise EOFError, "end of file reached" begin synchronize do case resp @@ -3566,8 +3558,22 @@ def receive_responses end end end + rescue Exception => exception + # NOTE: this rescue clause is only capturing the exception for the ensure + # clause. Using `$!` in the ensure clause seems to trigger a weird + # TruffleRuby bug: https://github.com/truffleruby/truffleruby/issues/4308 + # + # We don't assign @exception directly here, because we want that to be + # atomically synchronized with all of the other changes in `ensure`. + raise ensure synchronize do + if exception + # Handling exceptions here, not in a rescue clause, so the lock isn't + # released and reacquired between rescue and ensure clauses. + @exception ||= exception + @sock.close + end state_logout! @receiver_thread_terminating = true @tagged_response_arrival.broadcast