on main help sign in
Merge pull request #37 from charmbracelet/few-concurrency-improvements
8a6988eRaphael Amorim
internal/permission/permission.go+31−7
@@ -1,10 +1,12 @@
1package permission
1package permission
2
2
3import (
3import (
4+ "context"
4 "errors"
5 "errors"
5 "path/filepath"
6 "path/filepath"
6 "slices"
7 "slices"
7 "sync"
8 "sync"
9+ "time"
8
10
9 "github.com/charmbracelet/crush/internal/config"
11 "github.com/charmbracelet/crush/internal/config"
10 "github.com/charmbracelet/crush/internal/pubsub"
12 "github.com/charmbracelet/crush/internal/pubsub"
@@ -44,9 +46,11 @@ type Service interface {
44type permissionService struct {
46type permissionService struct {
45 *pubsub.Broker[PermissionRequest]
47 *pubsub.Broker[PermissionRequest]
46
48
47− sessionPermissions []PermissionRequest
49+ sessionPermissions []PermissionRequest
50+ sessionPermissionsMu sync.RWMutex
48− pendingRequests sync.Map
51+ pendingRequests sync.Map
49− autoApproveSessions []string
52+ autoApproveSessions []string
53+ autoApproveSessionsMu sync.RWMutex
50}
54}
51
55
52func (s *permissionService) GrantPersistent(permission PermissionRequest) {
56func (s *permissionService) GrantPersistent(permission PermissionRequest) {
@@ -54,7 +58,10 @@ func (s *permissionService) GrantPersistent(permission PermissionRequest) {
54 if ok {
58 if ok {
55 respCh.(chan bool) <- true
59 respCh.(chan bool) <- true
56 }
60 }
61+
62+ s.sessionPermissionsMu.Lock()
57 s.sessionPermissions = append(s.sessionPermissions, permission)
63 s.sessionPermissions = append(s.sessionPermissions, permission)
64+ s.sessionPermissionsMu.Unlock()
58}
65}
59
66
60func (s *permissionService) Grant(permission PermissionRequest) {
67func (s *permissionService) Grant(permission PermissionRequest) {
@@ -72,9 +79,14 @@ func (s *permissionService) Deny(permission PermissionRequest) {
72}
79}
73
80
74func (s *permissionService) Request(opts CreatePermissionRequest) bool {
81func (s *permissionService) Request(opts CreatePermissionRequest) bool {
82+ s.autoApproveSessionsMu.RLock()
75− if slices.Contains(s.autoApproveSessions, opts.SessionID) {
83+ autoApprove := slices.Contains(s.autoApproveSessions, opts.SessionID)
84+ s.autoApproveSessionsMu.RUnlock()
85+
86+ if autoApprove {
76 return true
87 return true
77 }
88 }
89+
78 dir := filepath.Dir(opts.Path)
90 dir := filepath.Dir(opts.Path)
79 if dir == "." {
91 if dir == "." {
80 dir = config.WorkingDirectory()
92 dir = config.WorkingDirectory()
@@ -89,11 +101,14 @@ func (s *permissionService) Request(opts CreatePermissionRequest) bool {
89 Params: opts.Params,
101 Params: opts.Params,
90 }
102 }
91
103
104+ s.sessionPermissionsMu.RLock()
92 for _, p := range s.sessionPermissions {
105 for _, p := range s.sessionPermissions {
93 if p.ToolName == permission.ToolName && p.Action == permission.Action && p.SessionID == permission.SessionID && p.Path == permission.Path {
106 if p.ToolName == permission.ToolName && p.Action == permission.Action && p.SessionID == permission.SessionID && p.Path == permission.Path {
107+ s.sessionPermissionsMu.RUnlock()
94 return true
108 return true
95 }
109 }
96 }
110 }
111+ s.sessionPermissionsMu.RUnlock()
97
112
98 respCh := make(chan bool, 1)
113 respCh := make(chan bool, 1)
99
114
@@ -102,13 +117,22 @@ func (s *permissionService) Request(opts CreatePermissionRequest) bool {
102
117
103 s.Publish(pubsub.CreatedEvent, permission)
118 s.Publish(pubsub.CreatedEvent, permission)
104
119
105− // Wait for the response with a timeout
120+ // Wait for the response with a timeout to prevent indefinite blocking
121+ ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
122+ defer cancel()
123+
124+ select {
106− resp := <-respCh
125+ case resp := <-respCh:
107− return resp
126+ return resp
127+ case <-ctx.Done():
128+ return false // Timeout - deny by default
129+ }
108}
130}
109
131
110func (s *permissionService) AutoApproveSession(sessionID string) {
132func (s *permissionService) AutoApproveSession(sessionID string) {
133+ s.autoApproveSessionsMu.Lock()
111 s.autoApproveSessions = append(s.autoApproveSessions, sessionID)
134 s.autoApproveSessions = append(s.autoApproveSessions, sessionID)
135+ s.autoApproveSessionsMu.Unlock()
112}
136}
113
137
114func NewPermissionService() Service {
138func NewPermissionService() Service {
internal/pubsub/broker.go+2
@@ -111,6 +111,8 @@ func (b *Broker[T]) Publish(t EventType, payload T) {
111 select {
111 select {
112 case sub <- event:
112 case sub <- event:
113 default:
113 default:
114+ // Channel is full, subscriber is slow - skip this event
115+ // This prevents blocking the publisher
114 }
116 }
115 }
117 }
116}
118}