From 13939379042452fff0b2402e519add22d9383dac Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Mon, 1 Feb 2021 20:26:15 -0800 Subject: [PATCH] Fix race condition and flawed logic mis-querying panel for servers; closes pterodactyl/panel#3059 --- remote/servers.go | 28 ++++++++++++++-------------- server/manager.go | 8 ++++---- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/remote/servers.go b/remote/servers.go index 150ae52..21b004d 100644 --- a/remote/servers.go +++ b/remote/servers.go @@ -71,31 +71,31 @@ func (c *client) GetServersPaged(ctx context.Context, page, limit int) ([]api.Ra return r.Data, r.Meta, nil } -func (c *client) GetServers(ctx context.Context, perPage int) ([]api.RawServerData, error) { - servers, pageMeta, err := c.GetServersPaged(ctx, 0, perPage) +// GetServers returns all of the servers that are present on the Panel making +// parallel API calls to the endpoint if more than one page of servers is returned. +func (c *client) GetServers(ctx context.Context, limit int) ([]api.RawServerData, error) { + servers, meta, err := c.GetServersPaged(ctx, 0, limit) if err != nil { return nil, err } - // if the amount of servers exceeds the page limit, get the remaining pages in parallel - if pageMeta.LastPage > 1 { - eg, _ := errgroup.WithContext(ctx) - serversMu := sync.Mutex{} - - for page := pageMeta.CurrentPage + 1; page <= pageMeta.LastPage; page++ { - eg.Go(func() error { - ps, _, err := c.GetServersPaged(ctx, perPage, int(page)) + var mu sync.Mutex + if meta.LastPage > 1 { + g, ctx := errgroup.WithContext(ctx) + for page := meta.CurrentPage + 1; page <= meta.LastPage; page++ { + page := page + g.Go(func() error { + ps, _, err := c.GetServersPaged(ctx, int(page), limit) if err != nil { return err } - serversMu.Lock() + mu.Lock() servers = append(servers, ps...) - serversMu.Unlock() + mu.Unlock() return nil }) } - - if err := eg.Wait(); err != nil { + if err := g.Wait(); err != nil { return nil, err } } diff --git a/server/manager.go b/server/manager.go index 1798ec0..0816a18 100644 --- a/server/manager.go +++ b/server/manager.go @@ -28,11 +28,11 @@ type Manager struct { // the servers that are currently present on the filesystem and set them into // the manager. func NewManager(ctx context.Context, client remote.Client) (*Manager, error) { - c := NewEmptyManager() - if err := c.initializeFromRemoteSource(ctx, client); err != nil { + m := NewEmptyManager() + if err := m.init(ctx, client); err != nil { return nil, err } - return c, nil + return m, nil } // NewEmptyManager returns a new empty manager collection without actually @@ -44,7 +44,7 @@ func NewEmptyManager() *Manager { // initializeFromRemoteSource iterates over a given directory and loads all of // the servers listed before returning them to the calling function. -func (m *Manager) initializeFromRemoteSource(ctx context.Context, client remote.Client) error { +func (m *Manager) init(ctx context.Context, client remote.Client) error { log.Info("fetching list of servers from API") servers, err := client.GetServers(ctx, config.Get().RemoteQuery.BootServersPerPage) if err != nil {