Skip to content

Commit e103d93

Browse files
toddbaerttoddbaert
authored
fix: more possible change deadlocks (#324)
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
1 parent 2b67574 commit e103d93

File tree

5 files changed

+172
-174
lines changed

5 files changed

+172
-174
lines changed

‎launchpad/handlers/http.go‎

Lines changed: 6 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,12 @@
11
package handlers
22

33
import (
4-
"context"
54
"encoding/json"
6-
"errors"
75
"fmt"
86
"net/http"
9-
"openfeature.com/flagd-testbed/launchpad/pkg"
10-
"os"
117
"strconv"
12-
"sync"
13-
"time"
8+
9+
flagd "openfeature.com/flagd-testbed/launchpad/pkg"
1410
)
1511

1612
// Response struct to standardize API responses
@@ -34,7 +30,7 @@ func StartFlagdHandler(w http.ResponseWriter, r *http.Request) {
3430
func RestartHandler(w http.ResponseWriter, r *http.Request) {
3531
secondsStr := r.URL.Query().Get("seconds")
3632
if secondsStr == "" {
37-
secondsStr = "5"
33+
secondsStr = fmt.Sprintf("%d", flagd.DefaultRestartTimeout)
3834
}
3935

4036
seconds, err := strconv.Atoi(secondsStr)
@@ -56,88 +52,14 @@ func StopFlagdHandler(w http.ResponseWriter, r *http.Request) {
5652
respondWithJSON(w, http.StatusOK, "success", "flagd stopped successfully")
5753
}
5854

59-
type FlagConfig struct {
60-
Flags map[string]struct {
61-
State string `json:"state"`
62-
Variants map[string]string `json:"variants"`
63-
DefaultVariant string `json:"defaultVariant"`
64-
} `json:"flags"`
65-
}
66-
67-
var mu sync.Mutex // Mutex to ensure thread-safe file operations
68-
6955
// ChangeHandler triggers JSON file merging and notifies `flagd`
7056
func ChangeHandler(w http.ResponseWriter, r *http.Request) {
71-
mu.Lock()
72-
defer mu.Unlock()
73-
74-
// Path to the configuration file
75-
configFile := "rawflags/changing-flag.json"
76-
77-
// Read the existing file
78-
data, err := os.ReadFile(configFile)
57+
variant, err := flagd.ToggleChangingFlag()
7958
if err != nil {
80-
http.Error(w, fmt.Sprintf("Failed to read file: %v", err), http.StatusInternalServerError)
59+
http.Error(w, fmt.Sprintf("Failed to toggle changing flag: %v", err), http.StatusInternalServerError)
8160
return
8261
}
83-
84-
// Parse the JSON into the FlagConfig struct
85-
var config FlagConfig
86-
if err := json.Unmarshal(data, &config); err != nil {
87-
http.Error(w, fmt.Sprintf("Failed to parse JSON: %v", err), http.StatusInternalServerError)
88-
return
89-
}
90-
91-
// Find the "changing-flag" and toggle the default variant
92-
flag, exists := config.Flags["changing-flag"]
93-
if !exists {
94-
http.Error(w, "Flag 'changing-flag' not found in the configuration", http.StatusNotFound)
95-
return
96-
}
97-
98-
// Toggle the defaultVariant between "foo" and "bar"
99-
if flag.DefaultVariant == "foo" {
100-
flag.DefaultVariant = "bar"
101-
} else {
102-
flag.DefaultVariant = "foo"
103-
}
104-
105-
// Save the updated flag back to the configuration
106-
config.Flags["changing-flag"] = flag
107-
// Serialize the updated configuration back to JSON
108-
updatedData, err := json.MarshalIndent(config, "", " ")
109-
if err != nil {
110-
http.Error(w, fmt.Sprintf("Failed to serialize updated JSON: %v", err), http.StatusInternalServerError)
111-
return
112-
}
113-
114-
// the file watcher should be triggered instantly. If not, we add a timeout to prevent a hanging test
115-
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
116-
defer cancel()
117-
118-
// wait for the filewatcher to register an update and write the new json file
119-
flagUpdateWait := sync.WaitGroup{}
120-
flagUpdateWait.Add(1)
121-
flagd.RegisterWaitForNextChangingFlagUpdate(&flagUpdateWait)
122-
go func() {
123-
flagUpdateWait.Wait()
124-
cancel()
125-
}()
126-
127-
// Write the updated JSON back to the file
128-
if err := os.WriteFile(configFile, updatedData, 0644); err != nil {
129-
http.Error(w, fmt.Sprintf("Failed to write updated file: %v", err), http.StatusInternalServerError)
130-
return
131-
}
132-
133-
select {
134-
case <-ctx.Done():
135-
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
136-
http.Error(w, fmt.Sprintf("Flags were not updated in time: %v", ctx.Err()), http.StatusInternalServerError)
137-
} else {
138-
respondWithJSON(w, http.StatusOK, "success", fmt.Sprintf("Default variant successfully changed to '%s'\n", flag.DefaultVariant))
139-
}
140-
}
62+
respondWithJSON(w, http.StatusOK, "success", fmt.Sprintf("Default variant successfully changed to '%s'\n", variant))
14163
}
14264

14365
// Utility function to send JSON responses

‎launchpad/pkg/file.go‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
package flagd
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"errors"
7+
"fmt"
8+
"os"
9+
"strings"
10+
"sync"
11+
"time"
12+
13+
"github.com/fsnotify/fsnotify"
14+
)
15+
16+
type FlagConfig struct {
17+
Flags map[string]struct {
18+
State string `json:"state"`
19+
Variants map[string]string `json:"variants"`
20+
DefaultVariant string `json:"defaultVariant"`
21+
} `json:"flags"`
22+
}
23+
24+
var (
25+
fileLock sync.Mutex // lock for file operations
26+
changeLock sync.Mutex // lock for change requests (so that multiple requests don't overlap)
27+
watcher *fsnotify.Watcher
28+
changeFlagUpdateListeners []*sync.WaitGroup
29+
)
30+
31+
func ToggleChangingFlag() (string, error) {
32+
changeLock.Lock()
33+
defer changeLock.Unlock()
34+
35+
// Path to the configuration file
36+
configFile := "rawflags/changing-flag.json"
37+
38+
// Read the existing file
39+
data, err := os.ReadFile(configFile)
40+
if err != nil {
41+
return "", err
42+
}
43+
44+
// Parse the JSON into the FlagConfig struct
45+
var config FlagConfig
46+
if err := json.Unmarshal(data, &config); err != nil {
47+
return "", err
48+
}
49+
50+
// Find the "changing-flag" and toggle the default variant
51+
flag, exists := config.Flags["changing-flag"]
52+
if !exists {
53+
return "", errors.New("changing-flag not found in configuration")
54+
}
55+
56+
// Toggle the defaultVariant between "foo" and "bar"
57+
if flag.DefaultVariant == "foo" {
58+
flag.DefaultVariant = "bar"
59+
} else {
60+
flag.DefaultVariant = "foo"
61+
}
62+
63+
// Save the updated flag back to the configuration
64+
config.Flags["changing-flag"] = flag
65+
// Serialize the updated configuration back to JSON
66+
updatedData, err := json.MarshalIndent(config, "", " ")
67+
if err != nil {
68+
return "", err
69+
}
70+
71+
// the file watcher should be triggered instantly. If not, we add a timeout to prevent a hanging test
72+
ctx, cancel := context.WithTimeout(context.Background(), time.Duration(DefaultRestartTimeout)*time.Second)
73+
defer cancel()
74+
75+
// wait for the filewatcher to register an update and write the new json file
76+
flagUpdateWait := sync.WaitGroup{}
77+
flagUpdateWait.Add(1)
78+
changeFlagUpdateListeners = append(changeFlagUpdateListeners, &flagUpdateWait)
79+
80+
fmt.Println("Waiting for flag update...")
81+
82+
go func() {
83+
flagUpdateWait.Wait()
84+
cancel()
85+
}()
86+
87+
// Write the updated JSON back to the file
88+
fileLock.Lock()
89+
if err := os.WriteFile(configFile, updatedData, 0644); err != nil {
90+
return "", err
91+
}
92+
fileLock.Unlock()
93+
94+
select {
95+
case <-ctx.Done():
96+
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
97+
return "", fmt.Errorf("Flags were not updated in time: %v", ctx.Err())
98+
} else {
99+
return flag.DefaultVariant, nil
100+
}
101+
}
102+
}
103+
104+
func RestartFileWatcher() error {
105+
fileLock.Lock()
106+
// grab old watcher while holding lock
107+
oldWatcher := watcher
108+
109+
// create new watcher
110+
var err error
111+
watcher, err = fsnotify.NewWatcher()
112+
if err != nil {
113+
fileLock.Unlock()
114+
return fmt.Errorf("failed to create file watcher: %v", err)
115+
}
116+
changeFlagUpdateListeners = []*sync.WaitGroup{}
117+
fileLock.Unlock()
118+
119+
if oldWatcher != nil {
120+
oldWatcher.Close()
121+
}
122+
123+
go func() {
124+
defer watcher.Close()
125+
for {
126+
select {
127+
case event, ok := <-watcher.Events:
128+
if !ok {
129+
return
130+
}
131+
if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 {
132+
fmt.Printf("%v config changed, regenerating JSON...\n", event.Name)
133+
if err := CombineJSONFiles(InputDir); err != nil {
134+
fmt.Printf("Error combining JSON files: %v\n", err)
135+
return
136+
}
137+
if strings.HasSuffix(event.Name, "changing-flag.json") {
138+
for _, v := range changeFlagUpdateListeners {
139+
v.Done()
140+
}
141+
changeFlagUpdateListeners = nil
142+
}
143+
}
144+
case err, ok := <-watcher.Errors:
145+
if !ok {
146+
return
147+
}
148+
fmt.Printf("File watcher error: %v\n", err)
149+
}
150+
}
151+
}()
152+
153+
if err := watcher.Add("./rawflags"); err != nil {
154+
return fmt.Errorf("failed to watch input directory: %v", err)
155+
}
156+
157+
fmt.Println("File watcher started.")
158+
return nil
159+
}

‎launchpad/pkg/filewatcher.go‎

Lines changed: 0 additions & 84 deletions
This file was deleted.

‎launchpad/pkg/flagd.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,11 @@ import (
1414
)
1515

1616
var (
17-
flagdCmd *exec.Cmd
18-
flagdLock sync.Mutex
19-
Config = "default"
20-
restartCancelFunc context.CancelFunc // Stores the cancel function for delayed restarts
17+
flagdCmd *exec.Cmd
18+
flagdLock sync.Mutex
19+
Config = "default"
20+
DefaultRestartTimeout = 5
21+
restartCancelFunc context.CancelFunc // Stores the cancel function for delayed restarts
2122
)
2223

2324
func ensureStartConditions() {

‎launchpad/pkg/json.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ var (
1818
)
1919

2020
func CombineJSONFiles(inputDir string) error {
21-
mu.Lock()
22-
defer mu.Unlock()
21+
fileLock.Lock()
22+
defer fileLock.Unlock()
2323

2424
files, err := os.ReadDir(inputDir)
2525
if err != nil {

0 commit comments

Comments
 (0)