Skip to content

Commit 5872768

Browse files
Fix concurrency errors, memory leaks, and add rate limiting (#1)
* Fix concurrency errors, memory leaks, and add rate limiting - Added mutexes (sync.RWMutex/sync.Mutex) to protect global shared state (zone, measures, conditions, themes, current weather client). - Fixed a memory leak in the rate limiter by adding a background cleanup goroutine for inactive IPs. - Improved DoS protection by adding a global rate limiter (100 req/s) in addition to the per-IP limiter. - Fixed an initialization bug in `loadConditions` using `sync.Once` and properly storing the error for subsequent calls. - Resolved a shadowing bug in `getDataPoints` and corrected its error message. - Configured SQLite with `MaxOpenConns(1)` to prevent database locking issues. - Ensured thread-safe access to templates and themes in `getServeMux` using `sync.Once`. Co-authored-by: birabittoh <26506860+birabittoh@users.noreply.github.com> * Fix concurrency errors, memory leaks, and add IP-based rate limiting with LRU cache - Implemented IP-based rate limiting using `github.com/hashicorp/golang-lru/v2` to bound memory usage and prevent leaks. - Added a global rate limiter for overall server protection. - Protected global shared state (zone, measures, conditions, themes, templates, current weather client) with mutexes and `sync.Once`. - Configured SQLite with `MaxOpenConns(1)` to avoid locking issues. - Fixed logical bugs in `getDataPoints` and `loadConditions`. - Updated `go.mod` and `go.sum` with the new LRU cache dependency. Co-authored-by: birabittoh <26506860+birabittoh@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: birabittoh <26506860+birabittoh@users.noreply.github.com>
1 parent 47679c1 commit 5872768

7 files changed

Lines changed: 120 additions & 39 deletions

File tree

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ require (
2525
github.com/glebarez/go-sqlite v1.22.0 // indirect
2626
github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0 // indirect
2727
github.com/google/uuid v1.6.0 // indirect
28+
github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
2829
github.com/jinzhu/inflection v1.0.0 // indirect
2930
github.com/jinzhu/now v1.1.5 // indirect
3031
github.com/mattn/go-isatty v0.0.20 // indirect

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e h1:ijClszYn+mADRFY17k
3737
github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e/go.mod h1:boTsfXsheKC2y+lKOCMpSfarhxDeIzfZG1jqGcPl3cA=
3838
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
3939
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
40+
github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
41+
github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
4042
github.com/jinzhu/inflection v1.0.0 h1:K317FqzuhWc8YvSVlFMCCUb36O/S9MCKRDI7QkRKD/E=
4143
github.com/jinzhu/inflection v1.0.0/go.mod h1:h+uFLlag+Qp1Va5pdKtLDYj+kHp5pxUVkryuEj+Srlc=
4244
github.com/jinzhu/now v1.1.5 h1:/o9tlHleP7gOFmsnYNz3RGnqzefHA47wQpKrrdTIwXQ=

src/api.go

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/url"
1010
"os"
1111
"strconv"
12+
"sync"
1213
"time"
1314

1415
bh "github.com/birabittoh/bunnyhue"
@@ -30,6 +31,7 @@ const (
3031

3132
var (
3233
tmpl map[string]*template.Template
34+
tmplOnce sync.Once
3335
funcMap = template.FuncMap{
3436
"capitalize": capitalize,
3537
"getHex": getHex,
@@ -44,7 +46,8 @@ var (
4446
"": &bh.Dark, // default
4547
"light": &bh.Light,
4648
}
47-
themes []string
49+
themes []string
50+
themesOnce sync.Once
4851
)
4952

5053
type PageData struct {
@@ -108,8 +111,12 @@ func getPageData(q url.Values, p *bh.Palette) (*PageData, error) {
108111

109112
now := time.Now()
110113

114+
funcMu.RLock()
115+
z := zone
116+
funcMu.RUnlock()
117+
111118
return &PageData{
112-
Zone: zone,
119+
Zone: z,
113120
Palette: p,
114121
FontFamily: fontFamily,
115122
OneWeekAgo: now.Add(-week).Unix(),
@@ -157,9 +164,18 @@ func getAPILatest(w http.ResponseWriter, r *http.Request) {
157164
}
158165

159166
func getAPIMeta(w http.ResponseWriter, r *http.Request) {
167+
funcMu.RLock()
168+
z := zone
169+
funcMu.RUnlock()
170+
171+
dbMu.RLock()
172+
m := make([]string, len(measures))
173+
copy(m, measures)
174+
dbMu.RUnlock()
175+
160176
respond(w, map[string]any{
161-
"zone": zone,
162-
"measures": measures,
177+
"zone": z,
178+
"measures": m,
163179
"themes": themes,
164180
})
165181
}
@@ -286,7 +302,13 @@ func getPlot(w http.ResponseWriter, r *http.Request) {
286302
if err != nil {
287303
http.Error(w, err.Error(), http.StatusInternalServerError)
288304
}
289-
pd.Measures = measures
305+
306+
dbMu.RLock()
307+
m := make([]string, len(measures))
308+
copy(m, measures)
309+
dbMu.RUnlock()
310+
311+
pd.Measures = m
290312
pd.Measure = r.PathValue("measure")
291313

292314
executeTemplateSafe(w, plotPath, pd)
@@ -298,15 +320,19 @@ func parseTemplate(path string) *template.Template {
298320

299321
func getServeMux() *http.ServeMux {
300322
// init templates
301-
tmpl = make(map[string]*template.Template)
323+
tmplOnce.Do(func() {
324+
tmpl = make(map[string]*template.Template)
302325

303-
tmpl[indexPath] = parseTemplate(indexPath)
304-
tmpl[recordsPath] = parseTemplate(recordsPath)
305-
tmpl[plotPath] = parseTemplate(plotPath)
326+
tmpl[indexPath] = parseTemplate(indexPath)
327+
tmpl[recordsPath] = parseTemplate(recordsPath)
328+
tmpl[plotPath] = parseTemplate(plotPath)
329+
})
306330

307-
for k := range palettes {
308-
themes = append(themes, k)
309-
}
331+
themesOnce.Do(func() {
332+
for k := range palettes {
333+
themes = append(themes, k)
334+
}
335+
})
310336

311337
// init conditions
312338
var err error

src/conditions.go

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,16 @@ import (
55
"log"
66
"os"
77
"strings"
8+
"sync"
89
"time"
910
)
1011

11-
var conditions map[string]Condition
12+
var (
13+
conditions map[string]Condition
14+
condOnce sync.Once
15+
condMu sync.RWMutex
16+
condErr error
17+
)
1218

1319
type Condition struct {
1420
Name string `json:"name"`
@@ -17,18 +23,22 @@ type Condition struct {
1723
}
1824

1925
func loadConditions() (map[string]Condition, error) {
20-
file, err := os.Open("conditions.json")
21-
if err != nil {
22-
return nil, err
23-
}
24-
defer file.Close()
26+
condOnce.Do(func() {
27+
file, err := os.Open("conditions.json")
28+
if err != nil {
29+
condErr = err
30+
return
31+
}
32+
defer file.Close()
2533

26-
err = json.NewDecoder(file).Decode(&conditions)
27-
if err != nil {
28-
return nil, err
29-
}
34+
condMu.Lock()
35+
condErr = json.NewDecoder(file).Decode(&conditions)
36+
condMu.Unlock()
37+
})
3038

31-
return conditions, nil
39+
condMu.RLock()
40+
defer condMu.RUnlock()
41+
return conditions, condErr
3242
}
3343

3444
func (record *Record) parseConditions() {
@@ -38,6 +48,8 @@ func (record *Record) parseConditions() {
3848

3949
weatherIDs := strings.Split(record.Weather, ",")
4050

51+
condMu.RLock()
52+
defer condMu.RUnlock()
4153
for _, w := range weatherIDs {
4254
c, ok := conditions[w]
4355
if !ok {

src/db.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const (
2424
var (
2525
db *gorm.DB
2626
measures []string
27+
dbMu sync.RWMutex
2728

2829
recordsCache = myks.New[[]Record](30 * time.Minute)
2930
dpCache = myks.New[[]DataPoint](31 * time.Minute)
@@ -129,20 +130,23 @@ func getLatestRecord() (record Record, err error) {
129130
return
130131
}
131132

132-
func getDataPoints(measures []string, f, t *int64) (dp []DataPoint, err error) {
133-
for _, measure := range measures {
133+
func getDataPoints(requestedMeasures []string, f, t *int64) (dp []DataPoint, err error) {
134+
dbMu.RLock()
135+
for _, measure := range requestedMeasures {
134136
if !slices.Contains(measures, measure) {
135-
err = errors.New("la misura richiesta non esiste")
137+
dbMu.RUnlock()
138+
err = errors.New("la misura richiesta non esiste: " + measure)
136139
return
137140
}
138141
}
142+
dbMu.RUnlock()
139143

140-
if len(measures) > 5 {
141-
err = errors.New("sono supportate al massimo 3 misure")
144+
if len(requestedMeasures) > 5 {
145+
err = errors.New("sono supportate al massimo 5 misure")
142146
return
143147
}
144148

145-
key := getKey(measures, f, t)
149+
key := getKey(requestedMeasures, f, t)
146150

147151
value, err := dpCache.Get(key)
148152
if err == nil {
@@ -151,7 +155,7 @@ func getDataPoints(measures []string, f, t *int64) (dp []DataPoint, err error) {
151155
}
152156

153157
selectText := "dt"
154-
for i, measure := range measures {
158+
for i, measure := range requestedMeasures {
155159
selectText += ", " + measure + " as value" + strconv.Itoa(i)
156160
}
157161

@@ -185,22 +189,33 @@ func initDB() (err error) {
185189
return errors.New("Errore nella migrazione del database: " + err.Error())
186190
}
187191

192+
// Limitazione delle connessioni per SQLite
193+
sqlDB, err := db.DB()
194+
if err == nil {
195+
sqlDB.SetMaxOpenConns(1)
196+
}
197+
188198
// Inizializzazione delle colonne
189199
s, err := schema.Parse(&Record{}, &sync.Map{}, schema.NamingStrategy{})
190200
if err != nil {
191201
return errors.New("Errore nel parsing dello schema: " + err.Error())
192202
}
203+
dbMu.Lock()
204+
measures = nil // Reset in case initDB is called multiple times
193205
for _, field := range s.Fields {
194206
if field.DBName == "" || field.DBName == "weather" || field.DBName == "dt" {
195207
continue
196208
}
197209
measures = append(measures, field.DBName)
198210
}
211+
dbMu.Unlock()
199212

200213
// Inizializzazione della zona
201214
zoneBytes, zErr := os.ReadFile(zonePath)
202215
if zErr == nil {
216+
funcMu.Lock()
203217
zone = string(zoneBytes)
218+
funcMu.Unlock()
204219
}
205220

206221
return

src/func.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os"
88
"strconv"
99
"strings"
10+
"sync"
1011
"time"
1112

1213
"github.com/briandowns/openweathermap"
@@ -18,6 +19,7 @@ var (
1819
directions = []string{"↑", "↗", "→", "↘", "↓", "↙", "←", "↖"}
1920
percentages = []string{"○", "◔", "◑", "◕", "●"}
2021
current *openweathermap.CurrentWeatherData
22+
funcMu sync.RWMutex
2123
)
2224

2325
// ------------------------
@@ -61,6 +63,9 @@ func getKey(m []string, from, to *int64) string {
6163

6264
// fetchAndSaveWeather effettua la chiamata all'API, mappa i dati nei modelli e li salva nel database.
6365
func fetchAndSaveWeather(db *gorm.DB, coords *openweathermap.Coordinates) {
66+
funcMu.Lock()
67+
defer funcMu.Unlock()
68+
6469
// Chiamata all'API usando le coordinate specificate
6570
err := current.CurrentByCoordinates(coords)
6671
if err != nil {
@@ -105,6 +110,10 @@ func fetchAndSaveWeather(db *gorm.DB, coords *openweathermap.Coordinates) {
105110
}
106111

107112
zone = current.Name
113+
dbMu.Lock()
114+
recordsCache.Delete("latest")
115+
dbMu.Unlock()
116+
108117
if _, err := os.Stat(zonePath); os.IsNotExist(err) {
109118
err = os.WriteFile(zonePath, []byte(zone), 0644)
110119
if err != nil {
@@ -114,7 +123,6 @@ func fetchAndSaveWeather(db *gorm.DB, coords *openweathermap.Coordinates) {
114123
log.Println("File " + zonePath + " creato con successo")
115124
}
116125

117-
recordsCache.Delete("latest")
118126
log.Println("Record salvato")
119127
}
120128

src/limiter.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,49 @@
11
package src
22

33
import (
4+
"log"
45
"net"
56
"net/http"
67
"sync"
78

9+
lru "github.com/hashicorp/golang-lru/v2"
810
"golang.org/x/time/rate"
911
)
1012

13+
var (
14+
limiterCache *lru.Cache[string, *rate.Limiter]
15+
limiterOnce sync.Once
16+
// globalLimiter protects the server from overall high load.
17+
globalLimiter = rate.NewLimiter(rate.Limit(100), 200)
18+
)
19+
1120
func rateLimiterMiddleware(next http.Handler) http.Handler {
12-
// Mappa degli IP con il rispettivo rate limiter
13-
limiters := make(map[string]*rate.Limiter)
14-
var limiterMutex = &sync.Mutex{}
21+
limiterOnce.Do(func() {
22+
var err error
23+
limiterCache, err = lru.New[string, *rate.Limiter](1000)
24+
if err != nil {
25+
log.Fatalf("Errore nella creazione dell'LRU cache: %v", err)
26+
}
27+
})
1528

1629
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
30+
// Global rate limit
31+
if !globalLimiter.Allow() {
32+
http.Error(w, "Server busy", http.StatusServiceUnavailable)
33+
return
34+
}
35+
1736
ip, _, err := net.SplitHostPort(r.RemoteAddr)
1837
if err != nil {
1938
http.Error(w, "Can't find IP address", http.StatusInternalServerError)
2039
return
2140
}
2241

23-
limiterMutex.Lock()
24-
limiter, exists := limiters[ip]
25-
if !exists {
42+
limiter, ok := limiterCache.Get(ip)
43+
if !ok {
2644
limiter = rate.NewLimiter(3, 30) // 3 richieste al secondo, burst massimo di 30
27-
limiters[ip] = limiter
45+
limiterCache.Add(ip, limiter)
2846
}
29-
limiterMutex.Unlock()
3047

3148
if !limiter.Allow() {
3249
http.Error(w, "Too many requests", http.StatusTooManyRequests)

0 commit comments

Comments
 (0)