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

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

Author: cmaglie

Diff for: commands/core/install.go

@@ -26,18 +26,37 @@ import (
 )
 
 // PlatformInstall FIXMEDOC
-func PlatformInstall(ctx context.Context, req *rpc.PlatformInstallRequest,
-	downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) (*rpc.PlatformInstallResponse, error) {
+func PlatformInstall(
+	ctx context.Context,
+	req *rpc.PlatformInstallRequest,
+	downloadCB rpc.DownloadProgressCB,
+	taskCB rpc.TaskProgressCB,
+) (*rpc.PlatformInstallResponse, error) {
+	if err := platformInstall(ctx, req, downloadCB, taskCB); err != nil {
+		return nil, err
+	}
+	if err := commands.Init(&rpc.InitRequest{Instance: req.Instance}, nil); err != nil {
+		return nil, err
+	}
+	return &rpc.PlatformInstallResponse{}, nil
+}
 
+// platformInstall is the implementation of platform install
+func platformInstall(
+	ctx context.Context,
+	req *rpc.PlatformInstallRequest,
+	downloadCB rpc.DownloadProgressCB,
+	taskCB rpc.TaskProgressCB,
+) error {
 	pme, release := commands.GetPackageManagerExplorer(req)
 	if pme == nil {
-		return nil, &arduino.InvalidInstanceError{}
+		return &arduino.InvalidInstanceError{}
 	}
 	defer release()
 
 	version, err := commands.ParseVersion(req)
 	if err != nil {
-		return nil, &arduino.InvalidVersionError{Cause: err}
+		return &arduino.InvalidVersionError{Cause: err}
 	}
 
 	ref := &packagemanager.PlatformReference{
@@ -47,30 +66,26 @@ func PlatformInstall(ctx context.Context, req *rpc.PlatformInstallRequest,
 	}
 	platformRelease, tools, err := pme.FindPlatformReleaseDependencies(ref)
 	if err != nil {
-		return nil, &arduino.PlatformNotFoundError{Platform: ref.String(), Cause: err}
+		return &arduino.PlatformNotFoundError{Platform: ref.String(), Cause: err}
 	}
 
 	// Prerequisite checks before install
 	if platformRelease.IsInstalled() {
 		taskCB(&rpc.TaskProgress{Name: tr("Platform %s already installed", platformRelease), Completed: true})
-		return &rpc.PlatformInstallResponse{}, nil
+		return nil
 	}
 
 	if req.GetNoOverwrite() {
 		if installed := pme.GetInstalledPlatformRelease(platformRelease.Platform); installed != nil {
-			return nil, fmt.Errorf("%s: %s",
+			return fmt.Errorf("%s: %s",
 				tr("Platform %s already installed", installed),
 				tr("could not overwrite"))
 		}
 	}
 
 	if err := pme.DownloadAndInstallPlatformAndTools(platformRelease, tools, downloadCB, taskCB, req.GetSkipPostInstall()); err != nil {
-		return nil, err
-	}
-
-	if err := commands.Init(&rpc.InitRequest{Instance: req.Instance}, nil); err != nil {
-		return nil, err
+		return err
 	}
 
-	return &rpc.PlatformInstallResponse{}, nil
+	return nil
 }

Diff for: commands/core/uninstall.go

@@ -26,9 +26,20 @@ import (
 
 // PlatformUninstall FIXMEDOC
 func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, taskCB rpc.TaskProgressCB) (*rpc.PlatformUninstallResponse, error) {
+	if err := platformUninstall(ctx, req, taskCB); err != nil {
+		return nil, err
+	}
+	if err := commands.Init(&rpc.InitRequest{Instance: req.Instance}, nil); err != nil {
+		return nil, err
+	}
+	return &rpc.PlatformUninstallResponse{}, nil
+}
+
+// platformUninstall is the implementation of platform unistaller
+func platformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, taskCB rpc.TaskProgressCB) error {
 	pme, release := commands.GetPackageManagerExplorer(req)
 	if pme == nil {
-		return nil, &arduino.InvalidInstanceError{}
+		return &arduino.InvalidInstanceError{}
 	}
 	defer release()
 
@@ -39,22 +50,22 @@ func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, t
 	if ref.PlatformVersion == nil {
 		platform := pme.FindPlatform(ref)
 		if platform == nil {
-			return nil, &arduino.PlatformNotFoundError{Platform: ref.String()}
+			return &arduino.PlatformNotFoundError{Platform: ref.String()}
 		}
 		platformRelease := pme.GetInstalledPlatformRelease(platform)
 		if platformRelease == nil {
-			return nil, &arduino.PlatformNotFoundError{Platform: ref.String()}
+			return &arduino.PlatformNotFoundError{Platform: ref.String()}
 		}
 		ref.PlatformVersion = platformRelease.Version
 	}
 
 	platform, tools, err := pme.FindPlatformReleaseDependencies(ref)
 	if err != nil {
-		return nil, &arduino.NotFoundError{Message: tr("Can't find dependencies for platform %s", ref), Cause: err}
+		return &arduino.NotFoundError{Message: tr("Can't find dependencies for platform %s", ref), Cause: err}
 	}
 
 	if err := pme.UninstallPlatform(platform, taskCB); err != nil {
-		return nil, err
+		return err
 	}
 
 	for _, tool := range tools {
@@ -64,9 +75,5 @@ func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, t
 		}
 	}
 
-	if err := commands.Init(&rpc.InitRequest{Instance: req.Instance}, nil); err != nil {
-		return nil, err
-	}
-
-	return &rpc.PlatformUninstallResponse{}, nil
+	return nil
 }

Diff for: commands/core/upgrade.go

@@ -25,12 +25,31 @@ import (
 )
 
 // PlatformUpgrade FIXMEDOC
-func PlatformUpgrade(ctx context.Context, req *rpc.PlatformUpgradeRequest,
-	downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) (*rpc.PlatformUpgradeResponse, error) {
+func PlatformUpgrade(
+	ctx context.Context,
+	req *rpc.PlatformUpgradeRequest,
+	downloadCB rpc.DownloadProgressCB,
+	taskCB rpc.TaskProgressCB,
+) (*rpc.PlatformUpgradeResponse, error) {
+	if err := platformUpgrade(ctx, req, downloadCB, taskCB); err != nil {
+		return nil, err
+	}
+	if err := commands.Init(&rpc.InitRequest{Instance: req.Instance}, nil); err != nil {
+		return nil, err
+	}
+	return &rpc.PlatformUpgradeResponse{}, nil
+}
 
+// platformUpgrade implements the platform upgrade procedure
+func platformUpgrade(
+	ctx context.Context,
+	req *rpc.PlatformUpgradeRequest,
+	downloadCB rpc.DownloadProgressCB,
+	taskCB rpc.TaskProgressCB,
+) error {
 	pme, release := commands.GetPackageManagerExplorer(req)
 	if pme == nil {
-		return nil, &arduino.InvalidInstanceError{}
+		return &arduino.InvalidInstanceError{}
 	}
 	defer release()
 
@@ -40,12 +59,8 @@ func PlatformUpgrade(ctx context.Context, req *rpc.PlatformUpgradeRequest,
 		PlatformArchitecture: req.Architecture,
 	}
 	if err := pme.DownloadAndInstallPlatformUpgrades(ref, downloadCB, taskCB, req.GetSkipPostInstall()); err != nil {
-		return nil, err
-	}
-
-	if err := commands.Init(&rpc.InitRequest{Instance: req.Instance}, nil); err != nil {
-		return nil, err
+		return err
 	}
 
-	return &rpc.PlatformUpgradeResponse{}, nil
+	return nil
 }