fixes a data race in SerializeObject function

[p0lyn0mial]

What type of PR is this?
/kind bug

What this PR does / why we need it: It changes deferredResponseWriter to return after calling the Close() method.
Previously all sorts of errors including a data race were possible because deferredResponseWriter resets the writer and returns it to the pool. An attempt to write to a nil writer will lead to invalid memory address or nil pointer dereference. Sharing the same instance of deferredResponseWriter might lead to index out of range [43] with length 30 and recovered from err index > windowEnd errors

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[p0lyn0mial]

/assign @sttts @mfojtik

[sttts]

change looks good otherwise.

[liggitt]

The whole point of &deferredResponseWriter was to be able to write useful errors to the response if there was an encoding error. We should be able to determine if the error came from writing to the underlying http writer, and if not, we should continue trying to write an error to the http response

/hold

[liggitt]

edit: never mind, misread the error flow… was thrown off by the non-standard if err == nil early return pattern

/hold cancel

[sttts]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: p0lyn0mial, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• staging/src/k8s.io/apiserver/OWNERS [sttts]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[yogeek]

Hi @p0lyn0mial and @sttts ! Can this fix be backported to 1.19 or is the upgrade to 1.20 mandatory please ?

[p0lyn0mial]

Hi @p0lyn0mial and @sttts ! Can this fix be backported to 1.19 or is the upgrade to 1.20 mandatory please ?

it was already backported to 1.19 - #98393
this PR was targeting 1.20

[liggitt]

this was already backported to 1.18 and 1.19

[yogeek]

@p0lyn0mial @liggitt Oh ok, thank you for the quick reply.
So in my current 1.19.8 cluster, normally, the fix is already there ?

I ask this because I arrived here from this one : kubernetes-sigs/prometheus-adapter#292 and we are in 1.19.8 and still observe these errors in the api-server logs :

E0330 13:25:31.344690       1 writers.go:105] apiserver was unable to write a JSON response: http: Handler timeout

Additionnal note : we looked at apiserver logs to try to understand a recent increase in apiserver memory consumption that cause some of our masters to be "NotReady" lately.

[liggitt]

still observe these errors in the api-server logs :

E0330 13:25:31.344690       1 writers.go:105] apiserver was unable to write a JSON response: http: Handler timeout

Additionnal note : we looked at apiserver logs to try to understand a recent increase in apiserver memory consumption that cause some of our masters to be "NotReady" lately.

Those are unrelated to this issue. The symptoms of this issue are listed in the description:

An attempt to write to a nil writer will lead to invalid memory address or nil pointer dereference. Sharing the same instance of deferredResponseWriter might lead to index out of range [43] with length 30 and recovered from err index > windowEnd errors

[yogeek]

ok @liggitt thank you for your help