From 7dc78ecf4d05017366ff8ed8c92659707809c1ae Mon Sep 17 00:00:00 2001 From: Aarsh Shah Date: Tue, 15 Sep 2020 11:19:03 +0530 Subject: [PATCH 1/2] request context cancelled error --- graphsync.go | 7 +++++++ requestmanager/requestmanager_test.go | 7 ++++++- requestmanager/responsecollector.go | 4 ++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/graphsync.go b/graphsync.go index 4acdabd0..8b12462e 100644 --- a/graphsync.go +++ b/graphsync.go @@ -94,6 +94,13 @@ const ( RequestCancelled = ResponseStatusCode(35) ) +// RequestContextCancelledErr is an error message received on the error channel when the request context given by the user is cancelled/times out +type RequestContextCancelledErr struct{} + +func (e RequestContextCancelledErr) Error() string { + return "Request Context Cancelled" +} + // RequestFailedBusyErr is an error message received on the error channel when the peer is busy type RequestFailedBusyErr struct{} diff --git a/requestmanager/requestmanager_test.go b/requestmanager/requestmanager_test.go index c4325795..3cf6a611 100644 --- a/requestmanager/requestmanager_test.go +++ b/requestmanager/requestmanager_test.go @@ -199,8 +199,13 @@ func TestCancelRequestInProgress(t *testing.T) { testutil.VerifyEmptyResponse(requestCtx, t, returnedResponseChan1) td.blockChain.VerifyWholeChain(requestCtx, returnedResponseChan2) - testutil.VerifyEmptyErrors(requestCtx, t, returnedErrorChan1) + testutil.VerifyEmptyErrors(requestCtx, t, returnedErrorChan2) + + errors := testutil.CollectErrors(requestCtx, t, returnedErrorChan1) + require.Len(t, errors, 1) + _, ok := errors[0].(graphsync.RequestContextCancelledErr) + require.True(t, ok) } func TestCancelManagerExitsGracefully(t *testing.T) { diff --git a/requestmanager/responsecollector.go b/requestmanager/responsecollector.go index 33939062..993b15ce 100644 --- a/requestmanager/responsecollector.go +++ b/requestmanager/responsecollector.go @@ -80,6 +80,10 @@ func (rc *responseCollector) collectResponses( case <-rc.ctx.Done(): return case <-requestCtx.Done(): + select { + case <-rc.ctx.Done(): + case returnedErrors <- graphsync.RequestContextCancelledErr{}: + } return case err, ok := <-incomingErrors: if !ok { From 4c9b3c863031bfb211a5b83d62283b7756e3e092 Mon Sep 17 00:00:00 2001 From: Aarsh Shah Date: Tue, 15 Sep 2020 17:58:43 +0530 Subject: [PATCH 2/2] solve race in context cancellation --- requestmanager/responsecollector.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/requestmanager/responsecollector.go b/requestmanager/responsecollector.go index 993b15ce..766b9fed 100644 --- a/requestmanager/responsecollector.go +++ b/requestmanager/responsecollector.go @@ -88,6 +88,16 @@ func (rc *responseCollector) collectResponses( case err, ok := <-incomingErrors: if !ok { incomingErrors = nil + // even if the `incomingErrors` channel is closed without any error, + // the context could still have timed out in which case we need to inform the caller of the same. + select { + case <-requestCtx.Done(): + select { + case <-rc.ctx.Done(): + case returnedErrors <- graphsync.RequestContextCancelledErr{}: + } + default: + } } else { receivedErrors = append(receivedErrors, err) }