encrypted dir conns confuse LeaveStreamsUnattached

[arma]

(This is Mikeperry's bug)

When we launch an encrypted dir conn in directory.c via
connection_ap_make_link(), it'll put the stream in state
AP_CONN_STATE_CIRCUIT_WAIT. This means that even controllers
that set LeaveStreamsUnattached won't need to handle it,
since it's already being handled.

Bug #1: controllers don't know this, and can't distinguish
streams that are already being handled. Mike had a patch to
put an extra flag on the stream status line.

Bug #2: then if the stream causes a circuit to get created, and gets
attached to that circuit, but the circuit times out or otherwise
fails to fulfill the request, the stream will be handled by
connection_ap_detach_retriable() which will set it to state
AP_CONN_STATE_CONTROLLER_WAIT. And the controller doesn't realize
that this time it's supposed to handle it (and it probably shouldn't
need to handle it)

[Automatically added by flyspray2trac: Operating System: All]

[arma]

--- connection_edge.c   (revision 14577)
+++ connection_edge.c   (working copy)
@@ -549,7 +549,7 @@
 {
   control_event_stream_status(conn, STREAM_EVENT_FAILED_RETRIABLE, reason);
   conn->_base.timestamp_lastread = time(NULL);
-  if (! get_options()->LeaveStreamsUnattached) {
+  if (!get_options()->LeaveStreamsUnattached || conn->use_begindir) {
     conn->_base.state = AP_CONN_STATE_CIRCUIT_WAIT;
     circuit_detach_stream(TO_CIRCUIT(circ),conn);
     return connection_ap_handshake_attach_circuit(conn);

Is this adequate for solving #2? I think so. If we want to be more clever,
we could make that second check into its own function, named something like
connection_edge_chooses_circuits_automatically().

[nickm]

Sent to mikeperry wrt his patch for #1:

See also Robert Hogan's suggestion on or-talk as of Wed, 26 Mar 2008
19:22:30 +0000. I think it might be a little more general than this.
Haven't thought too hard about it though.

As for the code: it looks mostly fine, but I don't like the way that
"reason" is overloaded to sometimes hold a remap reason and sometimes
overloaded to hold this new stream-source thing. Just because they're
currently orthogonal doesn't mean it's a good idea to use the same
field for both.

Also, as a nitpick, we usually abbreviate connection as "conn", not
"con".

[mikeperry]

Just for the record, I think Robert Hogan's proposal is the way to go rather than my
initial patch. But I haven't had the time to grep through the source and find all the
locations for different purpose types...

[nickm]

So I don't forget, the URL is ​http://archives.seul.org/or/talk/Mar-2008/msg00207.html .

[nickm]

Fix for #2 applied and backported.

[nickm]

Part #1 implemented in svn and on 0.2.0.x branch.

[nickm]

flyspray2trac: bug closed.