From a8b4b3634b8bef6040601d970a7c178da72b114d Mon Sep 17 00:00:00 2001 From: Ophestra Umiker Date: Wed, 25 Sep 2024 01:17:38 +0900 Subject: [PATCH] dbus: use generalised helper.Helper for xdg-dbus-proxy Signed-off-by: Ophestra Umiker --- dbus/dbus.go | 15 +---- dbus/run.go | 117 ++++++------------------------------- internal/app/share.dbus.go | 12 ++-- 3 files changed, 27 insertions(+), 117 deletions(-) diff --git a/dbus/dbus.go b/dbus/dbus.go index 9c99a70..d68dcfa 100644 --- a/dbus/dbus.go +++ b/dbus/dbus.go @@ -4,8 +4,6 @@ import ( "errors" "fmt" "io" - "os" - "os/exec" "sync" "git.ophivana.moe/cat/fortify/helper" @@ -14,19 +12,12 @@ import ( // Proxy holds references to a xdg-dbus-proxy process, and should never be copied. // Once sealed, configuration changes will no longer be possible and attempting to do so will result in a panic. type Proxy struct { - cmd *exec.Cmd - - statP [2]*os.File - argsP [2]*os.File + helper *helper.Helper path string session [2]string system [2]string - wait *chan error - read *chan error - ready *chan bool - seal io.WriterTo lock sync.RWMutex } @@ -39,8 +30,8 @@ func (p *Proxy) String() string { p.lock.RLock() defer p.lock.RUnlock() - if p.cmd != nil { - return p.cmd.String() + if p.helper != nil { + return p.helper.String() } if p.seal != nil { diff --git a/dbus/run.go b/dbus/run.go index cc77606..a4c04dc 100644 --- a/dbus/run.go +++ b/dbus/run.go @@ -3,12 +3,13 @@ package dbus import ( "errors" "os" - "os/exec" + + "git.ophivana.moe/cat/fortify/helper" ) // Start launches the D-Bus proxy and sets up the Wait method. // ready should be buffered and should only be received from once. -func (p *Proxy) Start(ready *chan bool) error { +func (p *Proxy) Start(ready chan error) error { p.lock.Lock() defer p.lock.Unlock() @@ -16,76 +17,21 @@ func (p *Proxy) Start(ready *chan bool) error { return errors.New("proxy not sealed") } - // acquire pipes - if pr, pw, err := os.Pipe(); err != nil { - return err - } else { - p.statP[0], p.statP[1] = pr, pw - } - if pr, pw, err := os.Pipe(); err != nil { - return err - } else { - p.argsP[0], p.argsP[1] = pr, pw - } - - p.cmd = exec.Command(p.path, - // ExtraFiles: If non-nil, entry i becomes file descriptor 3+i. - "--fd=3", - "--args=4", + h := helper.New(p.seal, p.path, + // Helper: Args is always 3 and status if set is always 4. + "--args=3", + "--fd=4", ) - p.cmd.Env = []string{} - p.cmd.ExtraFiles = []*os.File{p.statP[1], p.argsP[0]} - p.cmd.Stdout = os.Stdout - p.cmd.Stderr = os.Stderr - if err := p.cmd.Start(); err != nil { + // xdg-dbus-proxy does not need to inherit the environment + h.Env = []string{} + + h.Stdout = os.Stdout + h.Stderr = os.Stderr + if err := h.StartNotify(ready); err != nil { return err } - statsP, argsP := p.statP[0], p.argsP[1] - - if _, err := p.seal.WriteTo(argsP); err != nil { - if err1 := p.cmd.Process.Kill(); err1 != nil { - panic(err1) - } - return err - } else { - if err = argsP.Close(); err != nil { - if err1 := p.cmd.Process.Kill(); err1 != nil { - panic(err1) - } - return err - } - } - - wait := make(chan error) - go func() { - // live out the lifespan of the process - wait <- p.cmd.Wait() - }() - - read := make(chan error) - go func() { - n, err := statsP.Read(make([]byte, 1)) - switch n { - case -1: - if err1 := p.cmd.Process.Kill(); err1 != nil { - panic(err1) - } - read <- err - case 0: - read <- err - case 1: - *ready <- true - read <- nil - default: - panic("unreachable") // unexpected read count - } - }() - - p.wait = &wait - p.read = &read - p.ready = ready - + p.helper = h return nil } @@ -94,41 +40,14 @@ func (p *Proxy) Wait() error { p.lock.RLock() defer p.lock.RUnlock() - if p.wait == nil || p.read == nil { - return errors.New("proxy not running") + if p.helper == nil { + return errors.New("proxy not started") } - defer func() { - if err1 := p.statP[0].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) { - panic(err1) - } - if err1 := p.statP[1].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) { - panic(err1) - } - - if err1 := p.argsP[0].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) { - panic(err1) - } - if err1 := p.argsP[1].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) { - panic(err1) - } - - }() - - select { - case err := <-*p.wait: - *p.ready <- false - return err - case err := <-*p.read: - if err != nil { - *p.ready <- false - return err - } - return <-*p.wait - } + return p.helper.Wait() } // Close closes the status file descriptor passed to xdg-dbus-proxy, causing it to stop. func (p *Proxy) Close() error { - return p.statP[0].Close() + return p.helper.Close() } diff --git a/internal/app/share.dbus.go b/internal/app/share.dbus.go index 43f196e..2902502 100644 --- a/internal/app/share.dbus.go +++ b/internal/app/share.dbus.go @@ -23,7 +23,6 @@ const ( var ( ErrDBusConfig = errors.New("dbus config not supplied") ErrDBusProxy = errors.New(xdgDBusProxy + " not found") - ErrDBusFault = errors.New(xdgDBusProxy + " did not start correctly") ) type ( @@ -98,12 +97,12 @@ func (seal *appSeal) shareDBus(config [2]*dbus.Config) error { func (tx *appSealTx) startDBus() error { // ready channel passed to dbus package - ready := make(chan bool, 1) + ready := make(chan error, 1) // used by waiting goroutine to notify process return tx.dbusWait = make(chan struct{}) // background dbus proxy start - if err := tx.dbus.Start(&ready); err != nil { + if err := tx.dbus.Start(ready); err != nil { return (*StartDBusError)(wrapError(err, "cannot start message bus proxy:", err)) } verbose.Println("starting message bus proxy:", tx.dbus) @@ -130,9 +129,10 @@ func (tx *appSealTx) startDBus() error { tx.dbusWait <- struct{}{} }() - // ready is false if the proxy process faulted - if !<-ready { - return (*StartDBusError)(wrapError(ErrDBusFault, "message bus proxy failed")) + // ready is not nil if the proxy process faulted + if err := <-ready; err != nil { + // note that err here is either an I/O related error or a predetermined unexpected behaviour error + return (*StartDBusError)(wrapError(err, "message bus proxy fault after start:", err)) } verbose.Println("message bus proxy ready")