Closescoder/internal#546
Closescoder/internal#543

Fixes a data race in the echo provisioner

What happened

If you're curious, two things:

• I was under the assumption that the incoming responses would always be under the ownership of the provisioner (no need to worry about synchronization or locking) because it would be coming from freshly decoded protobuf messages. Apparently this is very wrong, because we just call the echo provisioner functions directly using protobuf types in a bunch of tests. Serialization/deserialization doesn't happen anywhere in the process, we're just passing pointers around.

• The incoming responses are a mixture of values which could be interpreted as "transferring ownership", and a few global/"static" values that we have around for common scenarios. Mutating the unique "owned" responses is fine, but mutating those global helpers is where the data race was occurring. The code was checking to see if the plan was "empty" (as inlen(plan.Plan) == 0) and then replacing it with a "valid empty" plan (as in[]byte("{}"), empty JSON object). The problem with this is that there should not be a valid plan ifplan.Error is set. The "valid empty" plan was already added as a field to thePlanComplete static helper, but was intentionally omitted from a few other helpers that haveError values.

So with all that context, the fix is to just not mutateplan.Plan ifplan.Error is set.

Also, since in production environments the datawill actually be owned by this code, freshly deserialized from a protobuf message, I don't think there's any way this write could affect a real deployment.