[breaking] daemon: Fix concurrency and streamline access to PackageManager (#1828)

* legacy: Removed ToolsLoader step

It has been splitted and merged into HardwareLoader and TargetBoardResolver
that are more appropriate.

* Fixed some function comments

* Thread-safe protect access to instances map

* Removed state-altering methods from PackageManager

They have been moved into a Builder object that has the ability to build
a new PackageManager. This allows to clearly separate subrotuines that
actually change the status of the PackageManager from subroutines that
just need to query it.

* Created packagemanager.Explorer to query PackageManager data

The Explorer object can be see as a read-only "view" to the underlying
PackageManager: we may ask the PackageManager to create an Explorer on
itself. The returned explorer will held a read-lock on the
PackageManager until it's disposed.

This architecture should prevent unwanted changes on the PackageManager
while it's being used, and viceversa, when the PackageManager is updated
it should be guaranteed that no Explorers are reading it.

* PlatformInstall/Uninstall must release PackageManager.Explorer before calling commands.Init

Otherwise, since Init will try to take a write-lock, it will block
indefinitely.

* Moved commands.InstanceContainer -> rpc.InstanceCommand

* Created a coreInstancesContainer

This container will handle all the atomic access to the instances map.

* Made CoreInstance.PackageManager field private

* Moved the reminder of PackageManager functions to Explorer or Builder

* Now GetPackageManager accepts an rpc.InstanceCommand

It has also been deprecated in favor of GetPackageManagerExplorer.

* Now GetLibraryManager accepts an rpc.InstanceCommand

* Refactored automatic builtin-tool installation

* Added gRPC LibraryUpgrade call and fixed 'lib upgrade' command

* Explorer and Builder should not extend PackageManager

Previuosly the methods PackageManager.NewBuilder and
PackageManager.NewExplorer were available also on Builder and Explorer.

Now Builder and Explorer does not inherith these methods anymore,
avoiding trivial errors like the one fixed in this commit in the
builder_utils package.

* Updated documentation

* Apply suggestions from code review

Co-authored-by: per1234 <[email protected]>

Author: cmaglie

Diff for: arduino/cores/packagemanager/download.go

@@ -43,8 +43,8 @@ func (platform *PlatformReference) String() string {
 
 // FindPlatform returns the Platform matching the PlatformReference or nil if not found.
 // The PlatformVersion field of the reference is ignored.
-func (pm *PackageManager) FindPlatform(ref *PlatformReference) *cores.Platform {
-	targetPackage, ok := pm.Packages[ref.Package]
+func (pme *Explorer) FindPlatform(ref *PlatformReference) *cores.Platform {
+	targetPackage, ok := pme.packages[ref.Package]
 	if !ok {
 		return nil
 	}
@@ -56,8 +56,8 @@ func (pm *PackageManager) FindPlatform(ref *PlatformReference) *cores.Platform {
 }
 
 // FindPlatformRelease returns the PlatformRelease matching the PlatformReference or nil if not found
-func (pm *PackageManager) FindPlatformRelease(ref *PlatformReference) *cores.PlatformRelease {
-	platform := pm.FindPlatform(ref)
+func (pme *Explorer) FindPlatformRelease(ref *PlatformReference) *cores.PlatformRelease {
+	platform := pme.FindPlatform(ref)
 	if platform == nil {
 		return nil
 	}
@@ -70,8 +70,8 @@ func (pm *PackageManager) FindPlatformRelease(ref *PlatformReference) *cores.Pla
 
 // FindPlatformReleaseDependencies takes a PlatformReference and returns a set of items to download and
 // a set of outputs for non existing platforms.
-func (pm *PackageManager) FindPlatformReleaseDependencies(item *PlatformReference) (*cores.PlatformRelease, []*cores.ToolRelease, error) {
-	targetPackage, exists := pm.Packages[item.Package]
+func (pme *Explorer) FindPlatformReleaseDependencies(item *PlatformReference) (*cores.PlatformRelease, []*cores.ToolRelease, error) {
+	targetPackage, exists := pme.packages[item.Package]
 	if !exists {
 		return nil, nil, fmt.Errorf(tr("package %s not found"), item.Package)
 	}
@@ -94,22 +94,22 @@ func (pm *PackageManager) FindPlatformReleaseDependencies(item *PlatformReferenc
 	}
 
 	// replaces "latest" with latest version too
-	toolDeps, err := pm.Packages.GetPlatformReleaseToolDependencies(release)
+	toolDeps, err := pme.packages.GetPlatformReleaseToolDependencies(release)
 	if err != nil {
 		return nil, nil, fmt.Errorf(tr("getting tool dependencies for platform %[1]s: %[2]s"), release.String(), err)
 	}
 
 	// discovery dependencies differ from normal tool since we always want to use the latest
 	// available version for the platform package
-	discoveryDependencies, err := pm.Packages.GetPlatformReleaseDiscoveryDependencies(release)
+	discoveryDependencies, err := pme.packages.GetPlatformReleaseDiscoveryDependencies(release)
 	if err != nil {
 		return nil, nil, fmt.Errorf(tr("getting discovery dependencies for platform %[1]s: %[2]s"), release.String(), err)
 	}
 	toolDeps = append(toolDeps, discoveryDependencies...)
 
 	// monitor dependencies differ from normal tool since we always want to use the latest
 	// available version for the platform package
-	monitorDependencies, err := pm.Packages.GetPlatformReleaseMonitorDependencies(release)
+	monitorDependencies, err := pme.packages.GetPlatformReleaseMonitorDependencies(release)
 	if err != nil {
 		return nil, nil, fmt.Errorf(tr("getting monitor dependencies for platform %[1]s: %[2]s"), release.String(), err)
 	}
@@ -120,14 +120,14 @@ func (pm *PackageManager) FindPlatformReleaseDependencies(item *PlatformReferenc
 
 // DownloadToolRelease downloads a ToolRelease. If the tool is already downloaded a nil Downloader
 // is returned. Uses the given downloader configuration for download, or the default config if nil.
-func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, progressCB rpc.DownloadProgressCB) error {
+func (pme *Explorer) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, progressCB rpc.DownloadProgressCB) error {
 	resource := tool.GetCompatibleFlavour()
 	if resource == nil {
 		return &arduino.FailedDownloadError{
 			Message: tr("Error downloading tool %s", tool),
 			Cause:   errors.New(tr("no versions available for the current OS"))}
 	}
-	if err := resource.Download(pm.DownloadDir, config, tool.String(), progressCB); err != nil {
+	if err := resource.Download(pme.DownloadDir, config, tool.String(), progressCB); err != nil {
 		return &arduino.FailedDownloadError{
 			Message: tr("Error downloading tool %s", tool),
 			Cause:   err}
@@ -137,9 +137,9 @@ func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *d
 
 // DownloadPlatformRelease downloads a PlatformRelease. If the platform is already downloaded a
 // nil Downloader is returned.
-func (pm *PackageManager) DownloadPlatformRelease(platform *cores.PlatformRelease, config *downloader.Config, progressCB rpc.DownloadProgressCB) error {
+func (pme *Explorer) DownloadPlatformRelease(platform *cores.PlatformRelease, config *downloader.Config, progressCB rpc.DownloadProgressCB) error {
 	if platform.Resource == nil {
 		return &arduino.PlatformNotFoundError{Platform: platform.String()}
 	}
-	return platform.Resource.Download(pm.DownloadDir, config, platform.String(), progressCB)
+	return platform.Resource.Download(pme.DownloadDir, config, platform.String(), progressCB)
 }

Diff for: arduino/cores/packagemanager/identify.go

@@ -22,12 +22,12 @@ import (
 
 // IdentifyBoard returns a list of boards whose identification properties match the
 // provided ones.
-func (pm *PackageManager) IdentifyBoard(idProps *properties.Map) []*cores.Board {
+func (pme *Explorer) IdentifyBoard(idProps *properties.Map) []*cores.Board {
 	if idProps.Size() == 0 {
 		return []*cores.Board{}
 	}
 	foundBoards := []*cores.Board{}
-	for _, board := range pm.InstalledBoards() {
+	for _, board := range pme.InstalledBoards() {
 		if board.IsBoardMatchingIDProperties(idProps) {
 			foundBoards = append(foundBoards, board)
 		}