From f205bcae8f06f9f49e9917aba94f05b54cc2ad6c Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Sat, 8 Feb 2025 15:40:12 +0100 Subject: [PATCH 1/5] refactor: cleanup unused fields --- local/project/project.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/local/project/project.go b/local/project/project.go index 53ad6e9f..ac179b12 100644 --- a/local/project/project.go +++ b/local/project/project.go @@ -34,11 +34,9 @@ import ( // Project represents a PHP project type Project struct { - HTTP *lhttp.Server - PHPServer *php.Server - Logger zerolog.Logger - homeDir string - projectDir string + HTTP *lhttp.Server + PHPServer *php.Server + Logger zerolog.Logger } // New creates a new PHP project @@ -49,9 +47,7 @@ func New(c *Config) (*Project, error) { } passthru, err := realPassthru(documentRoot, c.Passthru) p := &Project{ - Logger: c.Logger.With().Str("source", "HTTP").Logger(), - homeDir: c.HomeDir, - projectDir: c.ProjectDir, + Logger: c.Logger.With().Str("source", "HTTP").Logger(), HTTP: &lhttp.Server{ DocumentRoot: documentRoot, Port: c.Port, From a2f79dfdbec8c2615f9f7e95f692a7311696ae8c Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Sat, 8 Feb 2025 18:27:43 +0100 Subject: [PATCH 2/5] fix: cleanup $HOME/.symfony5/tmp temporary directories --- commands/local_server_start.go | 4 ++++ local/php/executor.go | 30 +++++++++++++++++++----------- local/php/php_server.go | 4 ++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/commands/local_server_start.go b/commands/local_server_start.go index 5e24c816..224edb66 100644 --- a/commands/local_server_start.go +++ b/commands/local_server_start.go @@ -417,6 +417,10 @@ var localServerStartCmd = &console.Command{ return err } terminal.Eprintln("") + // wait for the PHP Server to be done cleaning up + if p.PHPServer != nil { + <-p.PHPServer.StoppedChan + } ui.Success("Stopped all processes successfully") } return nil diff --git a/local/php/executor.go b/local/php/executor.go index e259449d..a014ce45 100644 --- a/local/php/executor.go +++ b/local/php/executor.go @@ -163,7 +163,11 @@ func (e *Executor) DetectScriptDir() (string, error) { return e.scriptDir, nil } -// Config determines the right version of PHP depending on the configuration (+ its configuration) +// Config determines the right version of PHP depending on the configuration +// (+ its configuration). It also creates some symlinks to ease the integration +// with underlying tools that could try to run PHP. This is the responsability +// of the caller to clean those temporary files. One can call +// CleanupTemporaryDirectories to do so. func (e *Executor) Config(loadDotEnv bool) error { // reset environment e.environ = make([]string, 0) @@ -220,8 +224,10 @@ func (e *Executor) Config(loadDotEnv bool) error { // prepending the PHP directory in the PATH does not work well if the PHP binary is not named "php" (like php7.3 for instance) // in that case, we create a temp directory with a symlink // we also link php-config for pecl to pick up the right one (it is always looks for something called php-config) - phpDir := filepath.Join(cliDir, "tmp", xid.New().String(), "bin") - e.tempDir = phpDir + if e.tempDir == "" { + e.tempDir = filepath.Join(cliDir, "tmp", xid.New().String()) + } + phpDir := filepath.Join(e.tempDir, "bin") if err := os.MkdirAll(phpDir, 0755); err != nil { return err } @@ -284,6 +290,15 @@ func (e *Executor) Config(loadDotEnv bool) error { return err } +func (e *Executor) CleanupTemporaryDirectories() { + if e.iniDir != "" { + os.RemoveAll(e.iniDir) + } + if e.tempDir != "" { + os.RemoveAll(e.tempDir) + } +} + // Find composer depending on the configuration func (e *Executor) findComposer(extraBin string) (string, error) { if scriptDir, err := e.DetectScriptDir(); err == nil { @@ -312,14 +327,7 @@ func (e *Executor) Execute(loadDotEnv bool) int { fmt.Fprintln(os.Stderr, err) return 1 } - defer func() { - if e.iniDir != "" { - os.RemoveAll(e.iniDir) - } - if e.tempDir != "" { - os.RemoveAll(e.tempDir) - } - }() + defer e.CleanupTemporaryDirectories() cmd := execCommand(e.Args[0], e.Args[1:]...) environ := append(os.Environ(), e.environ...) gpathname := "PATH" diff --git a/local/php/php_server.go b/local/php/php_server.go index 44c8a001..19d3e23d 100644 --- a/local/php/php_server.go +++ b/local/php/php_server.go @@ -48,6 +48,7 @@ import ( type Server struct { Version *phpstore.Version logger zerolog.Logger + StoppedChan chan bool appVersion string homeDir string projectDir string @@ -79,6 +80,7 @@ func NewServer(homeDir, projectDir, documentRoot, passthru, appVersion string, l projectDir: projectDir, documentRoot: documentRoot, passthru: passthru, + StoppedChan: make(chan bool, 1), }, nil } @@ -195,6 +197,8 @@ func (p *Server) Start(ctx context.Context, pidFile *pid.PidFile) (*pid.PidFile, for _, path := range pathsToRemove { os.RemoveAll(path) } + e.CleanupTemporaryDirectories() + p.StoppedChan <- true }() return errors.Wrap(errors.WithStack(runner.Run()), "PHP server exited unexpectedly") From e95e7ad0f515b737deeb0a15ac9149518466de32 Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Sat, 8 Feb 2025 18:44:34 +0100 Subject: [PATCH 3/5] fix: cleanup additional temporary directories --- commands/local_server_start.go | 1 + local/php/fpm.go | 6 +----- local/php/php_builtin_server.go | 7 +------ local/php/php_server.go | 16 ++++++++-------- local/pid/pidfile.go | 13 +++++++++++++ 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/commands/local_server_start.go b/commands/local_server_start.go index 224edb66..af3479e4 100644 --- a/commands/local_server_start.go +++ b/commands/local_server_start.go @@ -421,6 +421,7 @@ var localServerStartCmd = &console.Command{ if p.PHPServer != nil { <-p.PHPServer.StoppedChan } + pidFile.CleanupDirectories() ui.Success("Stopped all processes successfully") } return nil diff --git a/local/php/fpm.go b/local/php/fpm.go index b1092d24..e783dcb3 100644 --- a/local/php/fpm.go +++ b/local/php/fpm.go @@ -114,9 +114,5 @@ env['LC_ALL'] = C } func (p *Server) fpmConfigFile() string { - path := filepath.Join(p.homeDir, fmt.Sprintf("php/%s/fpm-%s.ini", name(p.projectDir), p.Version.Version)) - if _, err := os.Stat(filepath.Dir(path)); os.IsNotExist(err) { - _ = os.MkdirAll(filepath.Dir(path), 0755) - } - return path + return filepath.Join(p.tempDir, fmt.Sprintf("fpm-%s.ini", p.Version.Version)) } diff --git a/local/php/php_builtin_server.go b/local/php/php_builtin_server.go index 88638d75..3a394e55 100644 --- a/local/php/php_builtin_server.go +++ b/local/php/php_builtin_server.go @@ -21,7 +21,6 @@ package php import ( "fmt" - "os" "path/filepath" ) @@ -61,9 +60,5 @@ require $script; `) func (p *Server) phpRouterFile() string { - path := filepath.Join(p.homeDir, fmt.Sprintf("php/%s-router.php", name(p.projectDir))) - if _, err := os.Stat(filepath.Dir(path)); os.IsNotExist(err) { - _ = os.MkdirAll(filepath.Dir(path), 0755) - } - return path + return filepath.Join(p.tempDir, fmt.Sprintf("%s-router.php", p.Version.Version)) } diff --git a/local/php/php_server.go b/local/php/php_server.go index 19d3e23d..22224bb3 100644 --- a/local/php/php_server.go +++ b/local/php/php_server.go @@ -50,7 +50,7 @@ type Server struct { logger zerolog.Logger StoppedChan chan bool appVersion string - homeDir string + tempDir string projectDir string documentRoot string passthru string @@ -76,7 +76,6 @@ func NewServer(homeDir, projectDir, documentRoot, passthru, appVersion string, l Version: version, logger: logger.With().Str("source", "PHP").Str("php", version.Version).Str("path", version.ServerPath()).Logger(), appVersion: appVersion, - homeDir: homeDir, projectDir: projectDir, documentRoot: documentRoot, passthru: passthru, @@ -86,7 +85,13 @@ func NewServer(homeDir, projectDir, documentRoot, passthru, appVersion string, l // Start starts a PHP server func (p *Server) Start(ctx context.Context, pidFile *pid.PidFile) (*pid.PidFile, func() error, error) { - var pathsToRemove []string + p.tempDir = pidFile.TempDirectory() + if _, err := os.Stat(p.tempDir); os.IsNotExist(err) { + if err = os.MkdirAll(p.tempDir, 0755); err != nil { + return nil, nil, err + } + } + port, err := process.FindAvailablePort() if err != nil { p.logger.Debug().Err(err).Msg("unable to find an available port") @@ -125,7 +130,6 @@ func (p *Server) Start(ctx context.Context, pidFile *pid.PidFile) (*pid.PidFile, return nil, nil, errors.WithStack(err) } p.proxy.Transport = &cgiTransport{} - pathsToRemove = append(pathsToRemove, fpmConfigFile) binName = "php-fpm" workerName = "PHP-FPM" args = []string{p.Version.ServerPath(), "--nodaemonize", "--fpm-config", fpmConfigFile} @@ -151,7 +155,6 @@ func (p *Server) Start(ctx context.Context, pidFile *pid.PidFile) (*pid.PidFile, if err := os.WriteFile(routerPath, phprouter, 0644); err != nil { return nil, nil, errors.WithStack(err) } - pathsToRemove = append(pathsToRemove, routerPath) binName = "php" workerName = "PHP" args = []string{p.Version.ServerPath(), "-S", "127.0.0.1:" + strconv.Itoa(port), "-d", "variables_order=EGPCS", routerPath} @@ -194,9 +197,6 @@ func (p *Server) Start(ctx context.Context, pidFile *pid.PidFile) (*pid.PidFile, return phpPidFile, func() error { defer func() { - for _, path := range pathsToRemove { - os.RemoveAll(path) - } e.CleanupTemporaryDirectories() p.StoppedChan <- true }() diff --git a/local/pid/pidfile.go b/local/pid/pidfile.go index b5e41f28..243a2012 100644 --- a/local/pid/pidfile.go +++ b/local/pid/pidfile.go @@ -232,6 +232,19 @@ func (p *PidFile) WorkerPidDir() string { return filepath.Join(util.GetHomeDir(), "var", name(p.Dir)) } +func (p *PidFile) TempDirectory() string { + return filepath.Join(util.GetHomeDir(), "php", name(p.Dir)) +} + +func (p *PidFile) CleanupDirectories() { + os.RemoveAll(p.TempDirectory()) + // We don't want to force removal of log and pid files, we only want to + // clean up empty directories. To do so we use `os.Remove` instead of + // `os.RemoveAll` + os.Remove(p.WorkerLogDir()) + os.Remove(p.WorkerPidDir()) +} + func (p *PidFile) LogReader() (io.ReadCloser, error) { logFile := p.LogFile() if err := os.MkdirAll(filepath.Dir(logFile), 0755); err != nil { From f7b227d3534bb4136394ba0c3adca60e3a75147b Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Fri, 14 Feb 2025 05:54:12 -0500 Subject: [PATCH 4/5] Update local/php/executor.go Co-authored-by: Fabien Potencier --- local/php/executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/local/php/executor.go b/local/php/executor.go index a014ce45..b66dfaca 100644 --- a/local/php/executor.go +++ b/local/php/executor.go @@ -165,7 +165,7 @@ func (e *Executor) DetectScriptDir() (string, error) { // Config determines the right version of PHP depending on the configuration // (+ its configuration). It also creates some symlinks to ease the integration -// with underlying tools that could try to run PHP. This is the responsability +// with underlying tools that could try to run PHP. This is the responsibility // of the caller to clean those temporary files. One can call // CleanupTemporaryDirectories to do so. func (e *Executor) Config(loadDotEnv bool) error { From e0b9e03aefeb6b64b1a7e1d7d33ac35ad4bf51b9 Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Fri, 14 Feb 2025 13:33:53 +0100 Subject: [PATCH 5/5] fix: cleanup stale temporary directories please note that this might take a while because we clean by batches, use `find ~/.symfony5/tmp -type d -mindepth 1 -maxdepth 1 -delete` if you are eager --- local/php/composer.go | 5 +-- local/php/executor.go | 78 +++++++++++++++++++++++++++++++++++++++++ local/php/php_server.go | 1 + main.go | 2 ++ 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/local/php/composer.go b/local/php/composer.go index 65fd9ef8..c6f8b5b5 100644 --- a/local/php/composer.go +++ b/local/php/composer.go @@ -78,7 +78,7 @@ func Composer(dir string, args, env []string, stdout, stderr, logger io.Writer, fmt.Fprintln(logger, " WARNING: Unable to find Composer, downloading one. It is recommended to install Composer yourself at https://getcomposer.org/download/") // we don't store it under bin/ to avoid it being found by findComposer as we want to only use it as a fallback binDir := filepath.Join(util.GetHomeDir(), "composer") - if path, err = downloadComposer(binDir); err != nil { + if path, err = downloadComposer(binDir, debugLogger); err != nil { return ComposerResult{ code: 1, error: errors.Wrap(err, "unable to find composer, get it at https://getcomposer.org/download/"), @@ -157,7 +157,7 @@ func findComposer(extraBin string, logger zerolog.Logger) (string, error) { return "", os.ErrNotExist } -func downloadComposer(dir string) (string, error) { +func downloadComposer(dir string, debugLogger zerolog.Logger) (string, error) { if err := os.MkdirAll(dir, 0755); err != nil { return "", err } @@ -193,6 +193,7 @@ func downloadComposer(dir string) (string, error) { SkipNbArgs: 1, Stdout: &stdout, Stderr: &stdout, + Logger: debugLogger, } ret := e.Execute(false) if ret == 1 { diff --git a/local/php/executor.go b/local/php/executor.go index b66dfaca..91d384cd 100644 --- a/local/php/executor.go +++ b/local/php/executor.go @@ -29,6 +29,7 @@ import ( "runtime" "strings" "syscall" + "time" "github.com/pkg/errors" "github.com/rs/xid" @@ -291,6 +292,7 @@ func (e *Executor) Config(loadDotEnv bool) error { } func (e *Executor) CleanupTemporaryDirectories() { + go cleanupStaleTemporaryDirectories(e.Logger) if e.iniDir != "" { os.RemoveAll(e.iniDir) } @@ -299,6 +301,82 @@ func (e *Executor) CleanupTemporaryDirectories() { } } +// The Symfony CLI used to leak temporary directories until v5.10.8. The bug is +// fixed but because directories names are random they are not going to be +// reused and thus are not going to be cleaned up. And because they might be +// in-use by running servers we can't simply delete the parent directory. This +// is why we make our best to find the oldest directories and remove then, +// cleaning the directory little by little. +func cleanupStaleTemporaryDirectories(mainLogger zerolog.Logger) { + parentDirectory := filepath.Join(util.GetHomeDir(), "tmp") + mainLogger = mainLogger.With().Str("dir", parentDirectory).Logger() + + if len(parentDirectory) < 6 { + mainLogger.Warn().Msg("temporary dir path looks too short") + return + } + + mainLogger.Debug().Msg("Starting temporary directory cleanup...") + dir, err := os.Open(parentDirectory) + if err != nil { + mainLogger.Warn().Err(err).Msg("Failed to open temporary directory") + return + } + defer dir.Close() + + // the duration after which we consider temporary directories as + // stale and can be removed + cutoff := time.Now().Add(-7 * 24 * time.Hour) + + for { + // we might have a lof of entries so we need to work in batches + entries, err := dir.Readdirnames(30) + if err == io.EOF { + mainLogger.Debug().Msg("Cleaning is done...") + return + } + if err != nil { + mainLogger.Warn().Err(err).Msg("Failed to read entries") + return + } + + for _, entry := range entries { + logger := mainLogger.With().Str("entry", entry).Logger() + + // we generate temporary directory names with + // `xid.New().String()` which is always 20 char long + if len(entry) != 20 { + logger.Debug().Msg("found an entry that is not from us") + continue + } else if _, err := xid.FromString(entry); err != nil { + logger.Debug().Err(err).Msg("found an entry that is not from us") + continue + } + + entryPath := filepath.Join(parentDirectory, entry) + file, err := os.Open(entryPath) + if err != nil { + logger.Warn().Err(err).Msg("failed to read entry") + continue + } else if fi, err := file.Stat(); err != nil { + logger.Warn().Err(err).Msg("failed to read entry") + continue + } else if !fi.IsDir() { + logger.Warn().Err(err).Msg("entry is not a directory") + continue + } else if fi.ModTime().After(cutoff) { + logger.Debug().Any("cutoff", cutoff).Msg("entry is more recent than cutoff, keeping it for now") + continue + } + + logger.Debug().Str("entry", entry).Msg("entry matches the criterias, removing it") + if err := os.RemoveAll(entryPath); err != nil { + logger.Warn().Err(err).Msg("failed to remove entry") + } + } + } +} + // Find composer depending on the configuration func (e *Executor) findComposer(extraBin string) (string, error) { if scriptDir, err := e.DetectScriptDir(); err == nil { diff --git a/local/php/php_server.go b/local/php/php_server.go index 22224bb3..4384cc25 100644 --- a/local/php/php_server.go +++ b/local/php/php_server.go @@ -166,6 +166,7 @@ func (p *Server) Start(ctx context.Context, pidFile *pid.PidFile) (*pid.PidFile, BinName: binName, Args: args, scriptDir: p.projectDir, + Logger: p.logger, } p.logger.Info().Int("port", port).Msg("listening") diff --git a/main.go b/main.go index 2d89d1e9..4d93c765 100644 --- a/main.go +++ b/main.go @@ -68,12 +68,14 @@ func main() { BinName: args[1], Args: args[1:], ExtraEnv: getCliExtraEnv(), + Logger: terminal.Logger, } os.Exit(e.Execute(true)) } // called via "symfony console"? if len(args) >= 2 && args[1] == "console" { if executor, err := php.SymonyConsoleExecutor(args[2:]); err == nil { + executor.Logger = terminal.Logger executor.ExtraEnv = getCliExtraEnv() os.Exit(executor.Execute(false)) }