Skip to content

Commit fac959d

Browse files
authored
🐛 fix memory leak caused by gin context pooling. (konveyor#710)
The gin HTTP request handler uses a `gin.Context` pool. The _RichContext_ contains the response body which is passed to the `Render` middle-ware. We need to remove the rich context to prevent holding on to memory while the context is in the pool. _WithContext_ renamed to _RichContext_ for clarity. Signed-off-by: Jeff Ortel <[email protected]>
1 parent 8bf0223 commit fac959d

11 files changed

+52
-39
lines changed

api/analysis.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -2403,7 +2403,7 @@ func (r *IssueWriter) Create(id uint, filter qf.Filter) (path string, count int6
24032403

24042404
// db returns a db client.
24052405
func (r *IssueWriter) db() (db *gorm.DB) {
2406-
rtx := WithContext(r.ctx)
2406+
rtx := RichContext(r.ctx)
24072407
db = rtx.DB.Debug()
24082408
return
24092409
}
@@ -2477,7 +2477,7 @@ type AnalysisWriter struct {
24772477

24782478
// db returns a db client.
24792479
func (r *AnalysisWriter) db() (db *gorm.DB) {
2480-
rtx := WithContext(r.ctx)
2480+
rtx := RichContext(r.ctx)
24812481
db = rtx.DB.Debug()
24822482
return
24832483
}
@@ -2615,7 +2615,7 @@ type ReportWriter struct {
26152615

26162616
// db returns a db client.
26172617
func (r *ReportWriter) db() (db *gorm.DB) {
2618-
rtx := WithContext(r.ctx)
2618+
rtx := RichContext(r.ctx)
26192619
db = rtx.DB.Debug()
26202620
return
26212621
}

api/application.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func (h ApplicationHandler) Create(ctx *gin.Context) {
249249
return
250250
}
251251

252-
rtx := WithContext(ctx)
252+
rtx := RichContext(ctx)
253253
tr := trigger.Application{
254254
Trigger: trigger.Trigger{
255255
TaskManager: rtx.TaskManager,
@@ -388,7 +388,7 @@ func (h ApplicationHandler) Update(ctx *gin.Context) {
388388
}
389389
}
390390

391-
rtx := WithContext(ctx)
391+
rtx := RichContext(ctx)
392392
tr := trigger.Application{
393393
Trigger: trigger.Trigger{
394394
TaskManager: rtx.TaskManager,

api/auth.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ type Login struct {
9898
// been granted the necessary scope to access a resource.
9999
func Required(scope string) func(*gin.Context) {
100100
return func(ctx *gin.Context) {
101-
rtx := WithContext(ctx)
101+
rtx := RichContext(ctx)
102102
token := ctx.GetHeader(Authorization)
103103
request := &auth.Request{
104104
Token: token,

api/base.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@ type BaseHandler struct{}
3535

3636
// DB return db client associated with the context.
3737
func (h *BaseHandler) DB(ctx *gin.Context) (db *gorm.DB) {
38-
rtx := WithContext(ctx)
38+
rtx := RichContext(ctx)
3939
db = rtx.DB.Debug()
4040
return
4141
}
4242

4343
// Client returns k8s client from the context.
4444
func (h *BaseHandler) Client(ctx *gin.Context) (client client.Client) {
45-
rtx := WithContext(ctx)
45+
rtx := RichContext(ctx)
4646
client = rtx.Client
4747
return
4848
}
@@ -100,7 +100,7 @@ func (h *BaseHandler) pk(ctx *gin.Context) (id uint) {
100100

101101
// CurrentUser gets username from Keycloak auth token.
102102
func (h *BaseHandler) CurrentUser(ctx *gin.Context) (user string) {
103-
rtx := WithContext(ctx)
103+
rtx := RichContext(ctx)
104104
user = rtx.User
105105
if user == "" {
106106
Log.Info("Failed to get current user.")
@@ -113,7 +113,7 @@ func (h *BaseHandler) CurrentUser(ctx *gin.Context) (user string) {
113113
func (h *BaseHandler) HasScope(ctx *gin.Context, scope string) (b bool) {
114114
in := auth.BaseScope{}
115115
in.With(scope)
116-
rtx := WithContext(ctx)
116+
rtx := RichContext(ctx)
117117
for _, s := range rtx.Scopes {
118118
b = s.Match(in.Resource, in.Method)
119119
if b {
@@ -215,13 +215,13 @@ func (h *BaseHandler) Decoder(ctx *gin.Context, encoding string, r io.Reader) (d
215215

216216
// Status sets the status code.
217217
func (h *BaseHandler) Status(ctx *gin.Context, code int) {
218-
rtx := WithContext(ctx)
218+
rtx := RichContext(ctx)
219219
rtx.Status(code)
220220
}
221221

222222
// Respond sets the response.
223223
func (h *BaseHandler) Respond(ctx *gin.Context, code int, r any) {
224-
rtx := WithContext(ctx)
224+
rtx := RichContext(ctx)
225225
rtx.Respond(code, r)
226226
}
227227

api/batch.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func (h BatchHandler) create(ctx *gin.Context, create gin.HandlerFunc) {
6262
return
6363
}
6464

65-
rtx := WithContext(ctx)
65+
rtx := RichContext(ctx)
6666
bErr := BatchError{Message: "Create failed."}
6767
for i := range resources {
6868
b, _ := json.Marshal(resources[i])

api/context.go

+24-13
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ import (
1010
"sigs.k8s.io/controller-runtime/pkg/client"
1111
)
1212

13+
// Response values.
14+
type Response struct {
15+
Status int
16+
Body any
17+
}
18+
1319
// Context custom settings.
1420
type Context struct {
1521
*gin.Context
@@ -27,10 +33,15 @@ type Context struct {
2733
TaskManager *tasking.Manager
2834
}
2935

30-
// Response values.
31-
type Response struct {
32-
Status int
33-
Body any
36+
// Attach to gin context.
37+
func (r *Context) Attach(ctx *gin.Context) {
38+
r.Context = ctx
39+
ctx.Set("RichContext", r)
40+
}
41+
42+
// Detach from gin context
43+
func (r *Context) Detach() {
44+
delete(r.Context.Keys, "RichContext")
3445
}
3546

3647
// Status sets the values to respond to the request with.
@@ -42,24 +53,24 @@ func (r *Context) Status(status int) {
4253
}
4354

4455
// Respond sets the values to respond to the request with.
45-
func (r *Context) Respond(status int, body any) {
56+
func (r *Context) Respond(status int, body interface{}) {
4657
r.Response = Response{
4758
Status: status,
4859
Body: body,
4960
}
5061
}
5162

52-
// WithContext is a rich context.
53-
func WithContext(ctx *gin.Context) (n *Context) {
63+
// RichContext returns a rich context attached to the gin context.
64+
func RichContext(ctx *gin.Context) (rtx *Context) {
5465
key := "RichContext"
5566
object, found := ctx.Get(key)
5667
if !found {
57-
n = &Context{}
58-
ctx.Set(key, n)
68+
rtx = &Context{}
69+
rtx.Attach(ctx)
5970
} else {
60-
n = object.(*Context)
71+
rtx = object.(*Context)
6172
}
62-
n.Context = ctx
73+
rtx.Context = ctx
6374
return
6475
}
6576

@@ -70,7 +81,7 @@ func Transaction(ctx *gin.Context) {
7081
http.MethodPut,
7182
http.MethodPatch,
7283
http.MethodDelete:
73-
rtx := WithContext(ctx)
84+
rtx := RichContext(ctx)
7485
err := rtx.DB.Transaction(func(tx *gorm.DB) (err error) {
7586
db := rtx.DB
7687
rtx.DB = tx
@@ -93,7 +104,7 @@ func Transaction(ctx *gin.Context) {
93104
func Render() gin.HandlerFunc {
94105
return func(ctx *gin.Context) {
95106
ctx.Next()
96-
rtx := WithContext(ctx)
107+
rtx := RichContext(ctx)
97108
if rtx.Response.Body != nil {
98109
ctx.Negotiate(
99110
rtx.Response.Status,

api/error.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func ErrorHandler() gin.HandlerFunc {
8989

9090
err := ctx.Errors[0]
9191

92-
rtx := WithContext(ctx)
92+
rtx := RichContext(ctx)
9393
if errors.Is(err, &BadRequestError{}) ||
9494
errors.Is(err, &filter.Error{}) ||
9595
errors.Is(err, &sort.SortError{}) ||

api/identity.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func (h IdentityHandler) Update(ctx *gin.Context) {
208208
return
209209
}
210210

211-
rtx := WithContext(ctx)
211+
rtx := RichContext(ctx)
212212
tr := trigger.Identity{
213213
Trigger: trigger.Trigger{
214214
TaskManager: rtx.TaskManager,

api/task.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ func (h TaskHandler) Create(ctx *gin.Context) {
288288
_ = ctx.Error(err)
289289
return
290290
}
291-
rtx := WithContext(ctx)
291+
rtx := RichContext(ctx)
292292
task := &tasking.Task{}
293293
task.With(r.Model())
294294
task.CreateUser = h.BaseHandler.CurrentUser(ctx)
@@ -312,7 +312,7 @@ func (h TaskHandler) Create(ctx *gin.Context) {
312312
// @param id path int true "Task ID"
313313
func (h TaskHandler) Delete(ctx *gin.Context) {
314314
id := h.pk(ctx)
315-
rtx := WithContext(ctx)
315+
rtx := RichContext(ctx)
316316
err := rtx.TaskManager.Delete(h.DB(ctx), id)
317317
if err != nil {
318318
_ = ctx.Error(err)
@@ -355,7 +355,7 @@ func (h TaskHandler) Update(ctx *gin.Context) {
355355
m = r.Model()
356356
m.ID = id
357357
m.UpdateUser = h.CurrentUser(ctx)
358-
rtx := WithContext(ctx)
358+
rtx := RichContext(ctx)
359359
task := &tasking.Task{}
360360
task.With(m)
361361
err = rtx.TaskManager.Update(h.DB(ctx), task)
@@ -393,7 +393,7 @@ func (h TaskHandler) Submit(ctx *gin.Context) {
393393
// @param id path int true "Task ID"
394394
func (h TaskHandler) Cancel(ctx *gin.Context) {
395395
id := h.pk(ctx)
396-
rtx := WithContext(ctx)
396+
rtx := RichContext(ctx)
397397
err := rtx.TaskManager.Cancel(h.DB(ctx), id)
398398
if err != nil {
399399
_ = ctx.Error(err)

api/taskgroup.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (h TaskGroupHandler) Create(ctx *gin.Context) {
142142
_ = ctx.Error(result.Error)
143143
return
144144
}
145-
rtx := WithContext(ctx)
145+
rtx := RichContext(ctx)
146146
for i := range m.Tasks {
147147
task := &tasking.Task{}
148148
task.With(&m.Tasks[i])
@@ -234,7 +234,7 @@ func (h TaskGroupHandler) Update(ctx *gin.Context) {
234234
_ = ctx.Error(err)
235235
return
236236
}
237-
rtx := WithContext(ctx)
237+
rtx := RichContext(ctx)
238238
for i := range m.Tasks {
239239
task := &tasking.Task{}
240240
task.With(&m.Tasks[i])
@@ -273,7 +273,7 @@ func (h TaskGroupHandler) Delete(ctx *gin.Context) {
273273
_ = ctx.Error(err)
274274
return
275275
}
276-
rtx := WithContext(ctx)
276+
rtx := RichContext(ctx)
277277
for i := range m.Tasks {
278278
task := &m.Tasks[i]
279279
err = rtx.TaskManager.Delete(h.DB(ctx), task.ID)

cmd/main.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -198,15 +198,17 @@ func main() {
198198
}
199199
// Web
200200
router := gin.Default()
201-
router.Use(api.Render())
202-
router.Use(api.ErrorHandler())
203201
router.Use(
204202
func(ctx *gin.Context) {
205-
rtx := api.WithContext(ctx)
203+
rtx := api.RichContext(ctx)
204+
rtx.TaskManager = &taskManager
206205
rtx.DB = db
207206
rtx.Client = client
208-
rtx.TaskManager = &taskManager
207+
ctx.Next()
208+
rtx.Detach()
209209
})
210+
router.Use(api.Render())
211+
router.Use(api.ErrorHandler())
210212
for _, h := range api.All() {
211213
h.AddRoutes(router)
212214
}

0 commit comments

Comments
 (0)