Opened 2 years ago

Closed 5 months ago

#16248 closed defect (fixed)

[Assertion] connection_stop_reading: Assertion conn->read_event failed; aborting

Reported by: yurivict271 Owned by:
Priority: High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 027-backport, 2016-bug-retrospective
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

<skipped>

May 31 01:14:37.000 [notice] We tried for 15 seconds to connect to '[scrubbed]' using exit $EC116BCB80565A408CE67F8EC3FE3B0B02C3A065~orion at 94.242.246.24. Retrying on a new circuit.
May 31 01:14:37.000 [err] void tor_assertion_failed_(const char *, unsigned int, const char *, const char *)(): Bug: src/or/main.c:580: connection_stop_reading: Assertion conn->read_event failed; aborting.
May 31 01:14:37.000 [err] Bug: Assertion conn->read_event failed in connection_stop_reading at src/or/main.c:580. (Stack trace not available)

This tor instance is only connected to the TransPort, with DNS to DNSPort.

I will be happy to provide any other information if I can.

Child Tickets

Attachments (1)

tor-1.log.bz2 (172.7 KB) - added by yurivict271 2 years ago.
tor log

Download all attachments as: .zip

Change History (33)

Changed 2 years ago by yurivict271

tor log

comment:1 Changed 2 years ago by yurivict271

tor-0.2.6.7 on FreeBSD 10.1

comment:2 Changed 2 years ago by yurivict271

tor crashed after this assertion.

comment:3 Changed 2 years ago by yurivict271

Conditions that lead to the crash:

  • High traffic through many TCP connections to different IPs
  • Possibly a lot of failures to connect to particular IPs
  • All traffic goes through TransPort, forwarded there by firewall

comment:4 Changed 2 years ago by nickm

  • Component changed from - Select a component to Tor
  • Keywords 026-backport added
  • Milestone set to Tor: 0.2.7.x-final
  • Summary changed from [Assertion] to [Assertion] connection_stop_reading: Assertion conn->read_event failed; aborting

comment:5 Changed 2 years ago by yurivict271

This happened before. The chance of this happening is low, but after enough time this happens again and again under the similar conditions.

comment:6 Changed 2 years ago by nickm_mobile

Do you think you could get a stack trace here with gdb? And do you know if this happens on platforms besides freebsd?

comment:7 Changed 2 years ago by yurivict271

I didn't run it on any other OS, because vm-to-tor service that connects VM to tor is FreeBSD specific.

Does tor have the (maybe compile time) option to print stack on asserts? Because this would have been very useful here. I will try to patch tor to print stack into log using libexecinfo.

comment:8 Changed 2 years ago by nickm_mobile

Tor does have stack trace support, but it seems like it isn't working for you. If you would like to investigate, have a look at src/common/backtrace.c . (Or maybe stacktrace.c)

comment:9 Changed 2 years ago by yurivict271

On FreeBSD backtrace and backtrace_symbols_fd functions are in /usr/include/execinfo.h header, and in /usr/lib/libexecinfo.so lib, so checking for it with AC_CHECK_FUNCS (in configure.ac) produced the wrong result.

I think such line "AC_CHECK_LIB(execinfo, backtrace, BACKTRACE_LIBS=-lexecinfo, BACKTRACE_LIBS=)" is the right check. I will try to make a patch.

comment:10 Changed 2 years ago by nickm

A patch for that would be great.

comment:11 Changed 22 months ago by nickm

  • Keywords PostFreeze027 added

I'd merge patches for these for 0.2.7 if they come in on time. In some cases, that will require figuring out an as-yet-unsolved bugs.

comment:12 Changed 20 months ago by nickm

  • Keywords 027-backport added
  • Milestone changed from Tor: 0.2.7.x-final to Tor: 0.2.8.x-final

comment:13 Changed 17 months ago by nickm

  • Priority changed from Medium to High

comment:14 Changed 17 months ago by yurivict271

  • Severity set to Normal

I didn't see this one happen on FreeBSD with the latest versions.

comment:15 Changed 16 months ago by nickm

  • Status changed from new to needs_information

comment:16 Changed 16 months ago by nickm

  • Milestone changed from Tor: 0.2.8.x-final to Tor: 0.2.9.x-final

Throw most 0.2.8 "NEW" tickets into 0.2.9. I expect that many of them will subsequently get triaged out.

comment:17 Changed 16 months ago by yurivict271

I didn't see this happen again. Will report if I see this again.

comment:18 Changed 16 months ago by cypherpunks

wrong

Last edited 16 months ago by cypherpunks (previous) (diff)

comment:19 Changed 16 months ago by cypherpunks

wrong

Last edited 16 months ago by cypherpunks (previous) (diff)

comment:20 Changed 16 months ago by cypherpunks

 /** Tell the main loop to stop notifying <b>conn</b> of any read events. */
 MOCK_IMPL(void,
 connection_stop_reading,(connection_t *conn))
 {
   tor_assert(conn);
 
   IF_HAS_BUFFEREVENT(conn, {
       bufferevent_disable(conn->bufev, EV_READ);
       return;
   });
 
+  /* if dummy conn then no socket and no event, nothing to do here */
+  if (conn->type == CONN_TYPE_AP && TO_EDGE_CONN(conn)->is_dns_request) {
+    tor_assert(!conn->read_event);
+    return;
+  }
+
   tor_assert(conn->read_event);
 
   if (conn->linked) {
     conn->reading_from_linked_conn = 0;
     connection_stop_reading_from_linked_conn(conn);
   } else {
     if (event_del(conn->read_event))
       log_warn(LD_NET, "Error from libevent setting read event state for %d "
                "to unwatched: %s",
                (int)conn->s,
                tor_socket_strerror(tor_socket_errno(conn->s)));
   }
 }
 
 /** Tell the main loop to start notifying <b>conn</b> of any read events. */
 MOCK_IMPL(void,
 connection_start_reading,(connection_t *conn))
 {
   tor_assert(conn);
 
   IF_HAS_BUFFEREVENT(conn, {
       bufferevent_enable(conn->bufev, EV_READ);
       return;
   });
 
+  /* if dummy conn then no socket and no event, nothing to do here */
+  if (conn->type == CONN_TYPE_AP && TO_EDGE_CONN(conn)->is_dns_request) {
+    tor_assert(!conn->read_event);
+    return;
+  }
+
   tor_assert(conn->read_event);
 
   if (conn->linked) {
     conn->reading_from_linked_conn = 1;
     if (connection_should_read_from_linked_conn(conn))
       connection_start_reading_from_linked_conn(conn);
   } else {
     if (event_add(conn->read_event, NULL))
       log_warn(LD_NET, "Error from libevent setting read event state for %d "
                "to watched: %s",
                (int)conn->s,
                tor_socket_strerror(tor_socket_errno(conn->s)));
   }
 }
Last edited 16 months ago by cypherpunks (previous) (diff)

comment:21 Changed 16 months ago by cypherpunks

this
a simple html code can to trigger the assert

comment:22 Changed 16 months ago by nickm

  • Milestone changed from Tor: 0.2.9.x-final to Tor: 0.2.8.x-final
  • Status changed from needs_information to needs_review

comment:23 Changed 16 months ago by nickm

  • Keywords 026-backport PostFreeze027 removed

Looks reasonable to me.

Also, this isn't the first time we've had a bug of this kind. I've added another commit to replace the assertions with log messages.

Please see branch 'bug16248_027'.

comment:24 Changed 16 months ago by nickm

  • Keywords must-fix-before-028-rc added

Marking these as must-fix-before-028-rc.

Actually, some of them may not need to get 'fixed' before the rc, but I believe that they should either get fixed, or we should have a good explanation of why they don't need to get fixed.

comment:25 Changed 16 months ago by dgoulet

I would love to have a comment on connection_check_event here because by reading the patch I can understand that the connection must have a valid pointer event except for DNS request (???). Would be nice to explain why this exception. This comment has been removed with this commit but doesn't explain much anyway:

-  /* if dummy conn then no socket and no event, nothing to do here */

Else, lgtm!

comment:26 Changed 16 months ago by nickm

  • Keywords must-fix-before-028-rc removed
  • Milestone changed from Tor: 0.2.8.x-final to Tor: 0.2.7.x-final

okay. added some comments to bug16248_027. Merged it to master. Calling it a backport candidate for 0.2.7

comment:27 Changed 16 months ago by nickm

okay. added some comments to bug16248_027. Merged it to master. Calling it a backport candidate for 0.2.7.

comment:28 Changed 15 months ago by andrea

  • Resolution set to fixed
  • Status changed from needs_review to closed

Backport merged into maint-0.2.7.

comment:29 Changed 15 months ago by nickm

  • Keywords 2016-bug-retrospective added

Mark more tickets for bug retrospective based on hand-review of changelogs from 0.2.5 onwards.

comment:30 Changed 5 months ago by nickm

  • Milestone changed from Tor: 0.2.7.x-final to Tor: 0.2.4.x-final
  • Resolution fixed deleted
  • Status changed from closed to reopened

This is available for backport in "bug16248_024" -- we're reconsidering as part of our stable backport triage.

comment:31 Changed 5 months ago by nickm

  • Status changed from reopened to merge_ready

comment:32 Changed 5 months ago by nickm

  • Resolution set to fixed
  • Status changed from merge_ready to closed

backported to 0.2.4

Note: See TracTickets for help on using tickets.