[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #30344 [Core Tor/Tor]: conn_read_callback is called on connections that are marked for closed
#30344: conn_read_callback is called on connections that are marked for closed
--------------------------+------------------------------
Reporter: robgjansen | Owner: (none)
Type: defect | Status: new
Priority: Medium | Milestone:
Component: Core Tor/Tor | Version: Tor: 0.3.5.8
Severity: Normal | Resolution:
Keywords: | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
--------------------------+------------------------------
Comment (by robgjansen):
I wonder if the better place to stop reading on marked connections is
inside of `connection_mark_for_close_internal_`, which appears to be the
only place (outside of testing code) where the `conn->marked_for_close`
state variable is written (modified).
{{{
diff --git a/src/core/mainloop/connection.c
b/src/core/mainloop/connection.c
index f2a646c..e24d349 100644
--- a/src/core/mainloop/connection.c
+++ b/src/core/mainloop/connection.c
@@ -941,6 +941,12 @@ connection_mark_for_close_internal_, (connection_t
*conn,
* the number of seconds since last successful write, so
* we get our whole 15 seconds */
conn->timestamp_last_write_allowed = time(NULL);
+
+ /* We should never listen for read events on marked connections,
because
+ * we never try to actually read from the connection again. */
+ if (connection_is_reading(conn)) {
+ connection_stop_reading(conn);
+ }
}
/** Find each connection that has hold_open_until_flushed set to
}}}
An issue with this approach is even though we disable read events as
above, we might enable it somewhere else (which would be a bug).
To ensure that bug never occurs, we could add a check in
`connection_start_reading` so that we return if the connection is marked.
{{{
diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c
index e82c77a..7922156 100644
--- a/src/core/mainloop/mainloop.c
+++ b/src/core/mainloop/mainloop.c
@@ -641,6 +641,10 @@ connection_start_reading,(connection_t *conn))
return;
}
+ if (conn->marked_for_close) {
+ return;
+ }
+
if (conn->linked) {
conn->reading_from_linked_conn = 1;
if (connection_should_read_from_linked_conn(conn))
}}}
Or maybe the logic should be added to `connection_check_event`?
(Note that my diffs are on a custom branch and may not apply cleanly.)
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30344#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs