lunny/gitea

Archived

Fix race in log #16490

Merged

lunny merged 7 commits from fix-race-in-log into main

2021-07-20 19:09:29 +00:00

zeripath commented

2021-07-19 19:55:20 +00:00

Contributor

A race has been detected in #1441 relating to getting log levels.

This PR protects the GetLevel and GetStacktraceLevel calls with a mutex.

Signed-off-by: Andrew Thornton art27@cantab.net

A race has been detected in #1441 relating to getting log levels. This PR protects the GetLevel and GetStacktraceLevel calls with a mutex. Signed-off-by: Andrew Thornton <art27@cantab.net>

lunny approved these changes 2021-07-20 08:18:21 +00:00

lafriks (Migrated from github.com) reviewed 2021-07-20 08:43:58 +00:00

modules/log/event.go

lafriks (Migrated from github.com) commented

2021-07-20 08:43:58 +00:00

Owner

imho mutex should be RWMutex and Get methods use RLock and RUnlock

imho `mutex` should be `RWMutex` and Get methods use `RLock` and `RUnlock`

zeripath reviewed 2021-07-20 09:14:04 +00:00

modules/log/event.go

zeripath commented

2021-07-20 09:14:04 +00:00

Author

Contributor

The RWmutex requires two calls to lock and unlock and would be slower.

This is simply a lock around a single value. If performance were really an issue an atomic.Load* atomic.Store* might be quicker but a RWMutex isn't going to improve this.

The RWmutex requires two calls to lock and unlock and would be slower. This is simply a lock around a single value. If performance were really an issue an `atomic.Load*` `atomic.Store*` might be quicker but a RWMutex isn't going to improve this.

lafriks (Migrated from github.com) reviewed 2021-07-20 12:00:47 +00:00

modules/log/event.go

lafriks (Migrated from github.com) commented

2021-07-20 12:00:47 +00:00

Owner

What do you mean two calls? It requires same number of calls but state read only calls is not locking each other

What do you mean two calls? It requires same number of calls but state read only calls is not locking each other

lafriks (Migrated from github.com) approved these changes 2021-07-20 17:20:32 +00:00

lafriks (Migrated from github.com) left a comment

Owner

There was no need to rename variable but otherwise lgtm

There was no need to rename variable but otherwise lgtm

This repo is archived. You cannot comment on pull requests.

No reviewers

No Label

good first issue

hacktoberfest-accepted

kind/deployment

kind/deprecated

kind/enhancement

kind/moderation

kind/regression

kind/translation

kind/upstream-related

performance/bigrepo

performance/cpu

performance/memory

performance/speed

priority/critical

priority/medium

proposal/rejected

reviewed/confirmed

reviewed/duplicate

reviewed/invalid

reviewed/not-a-bug

reviewed/wontfix

status/needs-feedback

theme/authentication

theme/backup-restore

theme/federation

theme/migration

theme/timetracker

No Milestone

No project

No Assignees

2 Participants

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: lunny/gitea#16490

No description provided.