Skip to content

Commit

Permalink
Throw on received message if the responsible RequestOnConn has fini…
Browse files Browse the repository at this point in the history
…shed

Fixes #309.

When a RoC of a multi-node request finishes, it RoC stays registered in the
`RequestOnConnSet` until all other RoCs have finished, too. During that
time the client can receive a message for this request through this
connection. It then looks the RoC up in the RoCset, finds it and calls
`RequestOnConn.setReceivedPayload`, which attempts to resume the fiber -
but the fiber has terminated so it cannot be resumed, and an assertion
fails.
This is the case of a protocol error, the client has finished handling the
request while the node is still sending messages. Handle it by tracking
which of the RoCs in the RoCset have finished and throwing `ProtocolError`
when attempting to use one.
  • Loading branch information
david-eckardt-sociomantic committed Mar 15, 2018
1 parent 704196a commit 2b6afcd
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 14 deletions.
3 changes: 2 additions & 1 deletion src/swarm/neo/client/Connection.d
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,8 @@ public final class Connection: ConnectionBase
{
if (auto request = this.request_set.getRequest(id))
{
return request.getRequestOnConnForNode(this.node_address);
return request.getRequestOnConnForNode(this.node_address,
this.protocol_error_);
}
else
{
Expand Down
3 changes: 2 additions & 1 deletion src/swarm/neo/client/IRequestSet.d
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ interface IRequest
FinishedNotifier;

import swarm.neo.AddrPort;
IRequestOnConn getRequestOnConnForNode ( AddrPort node_address );
import swarm.neo.protocol.ProtocolError;
IRequestOnConn getRequestOnConnForNode ( AddrPort node_address, ProtocolError e );
}

/*******************************************************************************
Expand Down
18 changes: 11 additions & 7 deletions src/swarm/neo/client/RequestOnConnSet.d
Original file line number Diff line number Diff line change
Expand Up @@ -252,25 +252,29 @@ public struct RequestOnConnSet

/***************************************************************************
Decrements the `num_active` counter and returns whether it is now 0.
Registers `roc` as inactive, decrements the `num_active` counter and
returns whether the counter is now 0.
Params:
roc = the `RequestOnConn` instance that has finished
Returns:
true if `num_active` is 0
***************************************************************************/

public bool finished ( )
public bool finished ( RequestOnConn roc )
in
{
assert(this.num_active);
}
body
{
// TODO: Consider `this.all_nodes.remove(request_on_conn)` instead of
// using the `active` flag. Not doing this now for a patch release to
// avoid potentially introducing a bug if there is an unobvious reason
// why it isn't done.
request_on_conn.active = false;
// TODO: Consider `this.all_nodes.remove(roc)` instead of using the
// `active` flag. Not doing this now for a patch release to avoid
// potentially introducing a bug if there is an unobvious reason why it
// isn't done.
roc.active = false;
return --this.num_active == 0;
}

Expand Down
26 changes: 21 additions & 5 deletions src/swarm/neo/client/RequestSet.d
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ public final class RequestSet: IRequestSet
}
body
{
if ( this.getRequestOnConnForNode(connection.remote_address) !is null )
if ( this.request_on_conns.get(connection.remote_address) !is null )
return;

auto request_on_conn = this.request_on_conns.add(
Expand All @@ -452,21 +452,37 @@ public final class RequestSet: IRequestSet
/***********************************************************************
Obtains the handler of this request that is currently communicating
with a node, if any.
with a node, if any. Expects the handler to be active and throws `e`
if not.
Params:
node_address = the address of the node to look up the handler
for
e = the exception to throw if a handler was found that is not
active any more but has finished
Returns:
the handler of this request that is currently communicating with
the node or null if none is doing that right now.
Throws:
`ProtocolError` if a handler was found that is not active any
more but has finished.
***********************************************************************/

public RequestOnConn getRequestOnConnForNode ( AddrPort node_address )
public RequestOnConn getRequestOnConnForNode ( AddrPort node_address,
ProtocolError e )
{
return this.request_on_conns.get(node_address);
if (auto roc = this.request_on_conns.get(node_address))
{
if (roc.active)
return roc;
else
throw e.set("Unexpected incoming message");
}
else
return null;
}

/***********************************************************************
Expand Down Expand Up @@ -617,7 +633,7 @@ public final class RequestSet: IRequestSet

public void handlerFinished ( RequestOnConn request_on_conn )
{
if (this.request_on_conns.finished())
if (this.request_on_conns.finished(request_on_conn))
{
scope working_data_iter = new RequestWorkingData;
this.finished_notifier(this.context, working_data_iter);
Expand Down

0 comments on commit 2b6afcd

Please sign in to comment.