Skip to content

Fix concurrent access to libraries manager gRPC functions. #2480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion commands/core/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func PlatformDownload(ctx context.Context, req *rpc.PlatformDownloadRequest, dow
}
defer release()

version, err := commands.ParseVersion(req)
version, err := commands.ParseVersion(req.GetVersion())
if err != nil {
return nil, &cmderrors.InvalidVersionError{Cause: err}
}
Expand Down
2 changes: 1 addition & 1 deletion commands/core/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func PlatformInstall(ctx context.Context, req *rpc.PlatformInstallRequest, downl
}
defer release()

version, err := commands.ParseVersion(req)
version, err := commands.ParseVersion(req.GetVersion())
if err != nil {
return &cmderrors.InvalidVersionError{Cause: err}
}
Expand Down
66 changes: 36 additions & 30 deletions commands/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,17 +301,13 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
}

// Create library manager and add libraries directories
lm := librariesmanager.NewLibraryManager(
pme.IndexDir,
pme.DownloadDir,
)
_ = instances.SetLibraryManager(instance, lm) // should never fail
lmb := librariesmanager.NewBuilder()

// Load libraries
for _, pack := range pme.GetPackages() {
for _, platform := range pack.Platforms {
if platformRelease := pme.GetInstalledPlatformRelease(platform); platformRelease != nil {
lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{
PlatformRelease: platformRelease,
Path: platformRelease.GetLibrariesDir(),
Location: libraries.PlatformBuiltIn,
Expand All @@ -320,22 +316,33 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
}
}

if err := lm.LoadIndex(); err != nil {
indexFileName, err := globals.LibrariesIndexResource.IndexFileName()
if err != nil {
// should never happen
panic("failed getting libraries index file name: " + err.Error())
}
indexFile := pme.IndexDir.Join(indexFileName)

logrus.WithField("index", indexFile).Info("Loading libraries index file")
li, err := librariesindex.LoadIndex(indexFile)
if err != nil {
s := status.Newf(codes.FailedPrecondition, tr("Loading index file: %v"), err)
responseError(s)
li = librariesindex.EmptyIndex
}
instances.SetLibrariesIndex(instance, li)

if profile == nil {
// Add directories of libraries bundled with IDE
if bundledLibsDir := configuration.IDEBuiltinLibrariesDir(configuration.Settings); bundledLibsDir != nil {
lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{
Path: bundledLibsDir,
Location: libraries.IDEBuiltIn,
})
}

// Add libraries directory from config file
lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{
Path: configuration.LibrariesDir(configuration.Settings),
Location: libraries.User,
})
Expand All @@ -349,17 +356,14 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
if !libDir.IsDir() {
// Download library
taskCallback(&rpc.TaskProgress{Name: tr("Downloading library %s", libraryRef)})
libRelease := lm.Index.FindRelease(&librariesindex.Reference{
Name: libraryRef.Library,
Version: libraryRef.Version,
})
if libRelease == nil {
libRelease, err := li.FindRelease(libraryRef.Library, libraryRef.Version)
if err != nil {
taskCallback(&rpc.TaskProgress{Name: tr("Library %s not found", libraryRef)})
err := &cmderrors.LibraryNotFoundError{Library: libraryRef.Library}
responseError(err.ToRPCStatus())
continue
}
if err := libRelease.Resource.Download(lm.DownloadsDir, nil, libRelease.String(), downloadCallback, ""); err != nil {
if err := libRelease.Resource.Download(pme.DownloadDir, nil, libRelease.String(), downloadCallback, ""); err != nil {
taskCallback(&rpc.TaskProgress{Name: tr("Error downloading library %s", libraryRef)})
e := &cmderrors.FailedLibraryInstallError{Cause: err}
responseError(e.ToRPCStatus())
Expand All @@ -369,7 +373,7 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro

// Install library
taskCallback(&rpc.TaskProgress{Name: tr("Installing library %s", libraryRef)})
if err := libRelease.Resource.Install(lm.DownloadsDir, libRoot, libDir); err != nil {
if err := libRelease.Resource.Install(pme.DownloadDir, libRoot, libDir); err != nil {
taskCallback(&rpc.TaskProgress{Name: tr("Error installing library %s", libraryRef)})
e := &cmderrors.FailedLibraryInstallError{Cause: err}
responseError(e.ToRPCStatus())
Expand All @@ -378,16 +382,23 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
taskCallback(&rpc.TaskProgress{Completed: true})
}

lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{
Path: libRoot,
Location: libraries.User,
})
}
}

for _, status := range lm.RescanLibraries() {
logrus.WithError(status.Err()).Warnf("Error loading library")
// TODO: report as warning: responseError(err)
lm := lmb.Build()
_ = instances.SetLibraryManager(instance, lm) // should never fail

{
lmi, release := lm.NewInstaller()
for _, status := range lmi.RescanLibraries() {
logrus.WithError(status.Err()).Warnf("Error loading library")
// TODO: report as warning: responseError(err)
}
release()
}

// Refreshes the locale used, this will change the
Expand All @@ -409,23 +420,18 @@ func Destroy(ctx context.Context, req *rpc.DestroyRequest) (*rpc.DestroyResponse
// UpdateLibrariesIndex updates the library_index.json
func UpdateLibrariesIndex(ctx context.Context, req *rpc.UpdateLibrariesIndexRequest, downloadCB rpc.DownloadProgressCB) error {
logrus.Info("Updating libraries index")
lm, err := instances.GetLibraryManager(req.GetInstance())
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return err
}
indexDir := pme.IndexDir
release()

if err := lm.IndexFile.Parent().MkdirAll(); err != nil {
if err := indexDir.MkdirAll(); err != nil {
return &cmderrors.PermissionDeniedError{Message: tr("Could not create index directory"), Cause: err}
}

// Create a temp dir to stage all downloads
tmp, err := paths.MkTempDir("", "library_index_download")
if err != nil {
return &cmderrors.TempDirCreationFailedError{Cause: err}
}
defer tmp.RemoveAll()

if err := globals.LibrariesIndexResource.Download(lm.IndexFile.Parent(), downloadCB); err != nil {
if err := globals.LibrariesIndexResource.Download(indexDir, downloadCB); err != nil {
return err
}

Expand Down
55 changes: 51 additions & 4 deletions commands/internal/instances/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager"
"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesindex"
"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesmanager"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/arduino-cli/version"
Expand All @@ -17,6 +18,7 @@ import (
type coreInstance struct {
pm *packagemanager.PackageManager
lm *librariesmanager.LibrariesManager
li *librariesindex.Index
}

// instances contains all the running Arduino Core Services instances
Expand Down Expand Up @@ -60,6 +62,49 @@ func GetLibraryManager(inst *rpc.Instance) (*librariesmanager.LibrariesManager,
return i.lm, nil
}

// GetLibraryManagerExplorer returns the library manager Explorer for the given instance.
func GetLibraryManagerExplorer(inst *rpc.Instance) (*librariesmanager.Explorer, func(), error) {
lm, err := GetLibraryManager(inst)
if err != nil {
return nil, nil, err
}
lmi, release := lm.NewExplorer()
return lmi, release, nil
}

// GetLibraryManagerInstaller returns the library manager Installer for the given instance.
func GetLibraryManagerInstaller(inst *rpc.Instance) (*librariesmanager.Installer, func(), error) {
lm, err := GetLibraryManager(inst)
if err != nil {
return nil, nil, err
}
lmi, release := lm.NewInstaller()
return lmi, release, nil
}

// GetLibrariesIndex returns the library index for the given instance.
func GetLibrariesIndex(inst *rpc.Instance) (*librariesindex.Index, error) {
instancesMux.Lock()
defer instancesMux.Unlock()
i := instances[inst.GetId()]
if i == nil {
return nil, &cmderrors.InvalidInstanceError{}
}
return i.li, nil
}

// SetLibrariesIndex sets the library index for the given instance.
func SetLibrariesIndex(inst *rpc.Instance, li *librariesindex.Index) error {
instancesMux.Lock()
defer instancesMux.Unlock()
i := instances[inst.GetId()]
if i == nil {
return &cmderrors.InvalidInstanceError{}
}
i.li = li
return nil
}

// SetLibraryManager sets the library manager for the given instance.
func SetLibraryManager(inst *rpc.Instance, lm *librariesmanager.LibrariesManager) bool {
instancesMux.Lock()
Expand All @@ -74,16 +119,18 @@ func SetLibraryManager(inst *rpc.Instance, lm *librariesmanager.LibrariesManager

// Create a new *rpc.Instance ready to be initialized
func Create(dataDir, packagesDir, downloadsDir *paths.Path, extraUserAgent ...string) (*rpc.Instance, error) {
instance := &coreInstance{}

// Create package manager
userAgent := "arduino-cli/" + version.VersionInfo.VersionString
for _, ua := range extraUserAgent {
userAgent += " " + ua
}
tempDir := dataDir.Join("tmp")
instance.pm = packagemanager.NewBuilder(dataDir, packagesDir, downloadsDir, tempDir, userAgent).Build()
instance.lm = librariesmanager.NewLibraryManager(dataDir, downloadsDir)

instance := &coreInstance{
pm: packagemanager.NewBuilder(dataDir, packagesDir, downloadsDir, tempDir, userAgent).Build(),
lm: librariesmanager.NewBuilder().Build(),
li: librariesindex.EmptyIndex,
}

// Save instance
instancesMux.Lock()
Expand Down
26 changes: 20 additions & 6 deletions commands/lib/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ package lib
import (
"context"

"github.com/arduino/arduino-cli/commands"
"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/commands/internal/instances"
"github.com/arduino/arduino-cli/internal/arduino/httpclient"
"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesindex"
"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesmanager"
"github.com/arduino/arduino-cli/internal/i18n"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
"github.com/sirupsen/logrus"
)

Expand All @@ -35,34 +36,47 @@ var tr = i18n.Tr
func LibraryDownload(ctx context.Context, req *rpc.LibraryDownloadRequest, downloadCB rpc.DownloadProgressCB) (*rpc.LibraryDownloadResponse, error) {
logrus.Info("Executing `arduino-cli lib download`")

lm, err := instances.GetLibraryManager(req.GetInstance())
var downloadsDir *paths.Path
if pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance()); err != nil {
return nil, err
} else {
downloadsDir = pme.DownloadDir
release()
}

li, err := instances.GetLibrariesIndex(req.GetInstance())
if err != nil {
return nil, err
}

logrus.Info("Preparing download")

lib, err := findLibraryIndexRelease(lm.Index, req)
version, err := commands.ParseVersion(req.GetVersion())
if err != nil {
return nil, err
}

lib, err := li.FindRelease(req.GetName(), version)
if err != nil {
return nil, err
}

if err := downloadLibrary(lm, lib, downloadCB, func(*rpc.TaskProgress) {}, "download"); err != nil {
if err := downloadLibrary(downloadsDir, lib, downloadCB, func(*rpc.TaskProgress) {}, "download"); err != nil {
return nil, err
}

return &rpc.LibraryDownloadResponse{}, nil
}

func downloadLibrary(lm *librariesmanager.LibrariesManager, libRelease *librariesindex.Release,
func downloadLibrary(downloadsDir *paths.Path, libRelease *librariesindex.Release,
downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB, queryParameter string) error {

taskCB(&rpc.TaskProgress{Name: tr("Downloading %s", libRelease)})
config, err := httpclient.GetDownloaderConfig()
if err != nil {
return &cmderrors.FailedDownloadError{Message: tr("Can't download library"), Cause: err}
}
if err := libRelease.Resource.Download(lm.DownloadsDir, config, libRelease.String(), downloadCB, queryParameter); err != nil {
if err := libRelease.Resource.Download(downloadsDir, config, libRelease.String(), downloadCB, queryParameter); err != nil {
return &cmderrors.FailedDownloadError{Message: tr("Can't download library"), Cause: err}
}
taskCB(&rpc.TaskProgress{Completed: true})
Expand Down
Loading