[GitHub] [apisix-dashboard] bisakhmondal commented on a change in pull request #2054: fix: perform pid lookup for existing pid file

[GitBox <gi...@apache.org>]

bisakhmondal commented on a change in pull request #2054:
URL: https://github.com/apache/apisix-dashboard/pull/2054#discussion_r709901346



##########
File path: api/internal/utils/pid.go
##########
@@ -51,7 +94,7 @@ func ReadPID(filepath string) (int, error) {
 	if err != nil {
 		return -1, err
 	}
-	pid, err := strconv.Atoi(string(data))
+	pid, err := strconv.Atoi(string(bytes.Trim(data, " \n")))

Review comment:
       in go1.16 I was getting `invalid pid: strconv.Atoi: parsing "3041138\n": invalid syntax` parsing error ( newline error) without any newline at the end. 
   
   However blank space is not required.

##########
File path: api/internal/utils/pid.go
##########
@@ -18,19 +18,62 @@
 package utils
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"runtime"
 	"strconv"
+	"syscall"
+	"time"
+
+	"github.com/pkg/errors"
 )
 
 // WritePID write pid to the given file path.
 func WritePID(filepath string, forceStart bool) error {
-	if pid, err := ReadPID(filepath); err == nil {
-		if !forceStart {
-			return fmt.Errorf("Instance of Manager API maybe running with a pid %d. If not, please run Manager API with '-f' or '--force' flag\n", pid)
+	pidStatAction := func() error {
+		pid, err := ReadPID(filepath)
+		if err != nil {
+			return nil
+		}
+		// In unix system this always succeeds, doesn't ensure if the process at all exist or not.
+		proc, err := os.FindProcess(pid)
+		if err != nil {
+			return errors.Errorf("instance of Manager API maybe running with a pid %d. If not, please run Manager API with '-f' or '--force' flag. Error :%v\n", pid, err)

Review comment:
       Thanks for pointing it out. Updated

##########
File path: api/internal/utils/pid.go
##########
@@ -18,19 +18,62 @@
 package utils
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"runtime"
 	"strconv"
+	"syscall"
+	"time"
+
+	"github.com/pkg/errors"
 )
 
 // WritePID write pid to the given file path.
 func WritePID(filepath string, forceStart bool) error {
-	if pid, err := ReadPID(filepath); err == nil {
-		if !forceStart {
-			return fmt.Errorf("Instance of Manager API maybe running with a pid %d. If not, please run Manager API with '-f' or '--force' flag\n", pid)
+	pidStatAction := func() error {
+		pid, err := ReadPID(filepath)
+		if err != nil {
+			return nil
+		}
+		// In unix system this always succeeds, doesn't ensure if the process at all exist or not.
+		proc, err := os.FindProcess(pid)
+		if err != nil {
+			return errors.Errorf("instance of Manager API maybe running with a pid %d. If not, please run Manager API with '-f' or '--force' flag. Error :%v\n", pid, err)
+		}
+
+		// Traditional unix way to check the existence of the particular PID.
+		err = proc.Signal(syscall.Signal(0))
+		// No entry of the process in process control block.
+		if err != nil {
+			return nil
+		}
+
+		// Windows has a tendency to pick unused old PIDs and allocate that to new process, so we won't kill it.
+		if runtime.GOOS != "windows" && forceStart {
+			fmt.Println("Killing existing instance of Manager API")

Review comment:
       Cool




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org