Description
For short demo see this pull request: #2356
The underlying problem in both cases are mutable fields that can be changed in separate go routines.
A minimal fix for the mock implementation is a sentence in the docs. Note that the existing API hints on the defenestrations - basically yarpctest.ContextWithCall
takes a map of string to string for the headers. By default maps should not be mutated and once can reasonably say that this is expected.
For the production one this is suboptimal. We can start with warning about the behavior. The api seems to hint that the call is thread safe - for example Call.OriginalHeaders says it is returning a copy of headers. The only other API that returns a mutrable object can similarly be supposed it is returning a copy: Call.HeaderNames
Fixing this in a place outside the Call
implementation will be impossible. Therefore this needs to be pointed out in the docs.
It is possible to fix this by guarding the access to the ic.resHeaders
with a lock. The cost is more memory used plus some contention.
I will create another pull request with a fix using a single lock. I do not think going the full way of using a read-write lock is worth it - headers are not massively more read than mutated and it makes no sense to do that.
As promised - here is the propose fix: #2358
Another thing that may be pointed out in the documentation - Call
serves as global object for the whole context chain within a service. Changes made to the object will be visible to separate places in the call chain.