зеркало из https://github.com/golang/sys.git
unix: solaris/illumos Event Ports ENOENT cleanup
When we receive ENOENT from port_dissociate, we should clean up our map reference. To do so safely, we need a place to track the user cookies. This work is in support of fsnotify/fsnotify#371 Change-Id: Iae53ccc508d533941ad19df9051ca046262095ef Reviewed-on: https://go-review.googlesource.com/c/sys/+/380034 Trust: Tobias Klauser <tobias.klauser@gmail.com> Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit is contained in:
Nahum Shalman
Ian Lance Taylor
Родитель
a9b59b0215
Коммит
594fa53f00
|
|
@ -0,0 +1,179 @@
|
|||
// Copyright 2022 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
//go:build solaris
|
||||
// +build solaris
|
||||
|
||||
package unix
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"runtime"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func (e *EventPort) checkInternals(t *testing.T, fds, paths, cookies, pending int) {
|
||||
t.Helper()
|
||||
p, err := e.Pending()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to query how many events are pending")
|
||||
}
|
||||
if len(e.fds) != fds || len(e.paths) != paths || len(e.cookies) != cookies || p != pending {
|
||||
format := "| fds: %d | paths: %d | cookies: %d | pending: %d |"
|
||||
expected := fmt.Sprintf(format, fds, paths, cookies, pending)
|
||||
got := fmt.Sprintf(format, len(e.fds), len(e.paths), len(e.cookies), p)
|
||||
t.Errorf("Internal state mismatch\nfound: %s\nexpected: %s", got, expected)
|
||||
}
|
||||
}
|
||||
|
||||
// Regression test for DissociatePath returning ENOENT
|
||||
// This test is intended to create a linear worst
|
||||
// case scenario of events being associated and
|
||||
// fired but not consumed before additional
|
||||
// calls to dissociate and associate happen
|
||||
// This needs to be an internal test so that
|
||||
// we can validate the state of the private maps
|
||||
func TestEventPortDissociateAlreadyGone(t *testing.T) {
|
||||
port, err := NewEventPort()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create an EventPort")
|
||||
}
|
||||
defer port.Close()
|
||||
dir := t.TempDir()
|
||||
tmpfile, err := os.CreateTemp(dir, "eventport")
|
||||
if err != nil {
|
||||
t.Fatalf("unable to create a tempfile: %v", err)
|
||||
}
|
||||
path := tmpfile.Name()
|
||||
stat, err := os.Stat(path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure to Stat file: %v", err)
|
||||
}
|
||||
err = port.AssociatePath(path, stat, FILE_MODIFIED, "cookie1")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure associating file: %v", err)
|
||||
}
|
||||
// We should have 1 path registered and 1 cookie in the jar
|
||||
port.checkInternals(t, 0, 1, 1, 0)
|
||||
// The path is associated, let's delete it.
|
||||
err = os.Remove(path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure deleting file: %v", err)
|
||||
}
|
||||
// The file has been deleted, some sort of pending event is probably
|
||||
// queued in the kernel waiting for us to get it AND the kernel is
|
||||
// no longer watching for events on it. BUT... Because we haven't
|
||||
// consumed the event, this API thinks it's still watched:
|
||||
watched := port.PathIsWatched(path)
|
||||
if !watched {
|
||||
t.Errorf("unexpected result from PathIsWatched")
|
||||
}
|
||||
// Ok, let's dissociate the file even before reading the event.
|
||||
// Oh, ENOENT. I guess it's not associated any more
|
||||
err = port.DissociatePath(path)
|
||||
if err != ENOENT {
|
||||
t.Errorf("unexpected result dissociating a seemingly associated path we know isn't: %v", err)
|
||||
}
|
||||
// As established by the return value above, this should clearly be false now:
|
||||
// Narrator voice: in the first version of this API, it wasn't.
|
||||
watched = port.PathIsWatched(path)
|
||||
if watched {
|
||||
t.Errorf("definitively unwatched file still in the map")
|
||||
}
|
||||
// We should have nothing registered, but 1 pending event corresponding
|
||||
// to the cookie in the jar
|
||||
port.checkInternals(t, 0, 0, 1, 1)
|
||||
f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE, 0666)
|
||||
if err != nil {
|
||||
t.Fatalf("creating test file failed: %s", err)
|
||||
}
|
||||
err = f.Close()
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure closing file: %v", err)
|
||||
}
|
||||
stat, err = os.Stat(path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure to Stat file: %v", err)
|
||||
}
|
||||
c := "cookie2" // c is for cookie, that's good enough for me
|
||||
err = port.AssociatePath(path, stat, FILE_MODIFIED, c)
|
||||
if err != nil {
|
||||
t.Errorf("unexpected failure associating file: %v", err)
|
||||
}
|
||||
// We should have 1 registered path and its cookie
|
||||
// as well as a second cookie corresponding to the pending event
|
||||
port.checkInternals(t, 0, 1, 2, 1)
|
||||
|
||||
// Fire another event
|
||||
err = os.Remove(path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure deleting file: %v", err)
|
||||
}
|
||||
port.checkInternals(t, 0, 1, 2, 2)
|
||||
err = port.DissociatePath(path)
|
||||
if err != ENOENT {
|
||||
t.Errorf("unexpected result dissociating a seemingly associated path we know isn't: %v", err)
|
||||
}
|
||||
// By dissociating this path after deletion we ensure that the paths map is now empty
|
||||
// If we're not careful we could trigger a nil pointer exception
|
||||
port.checkInternals(t, 0, 0, 2, 2)
|
||||
|
||||
f, err = os.OpenFile(path, os.O_WRONLY|os.O_CREATE, 0666)
|
||||
if err != nil {
|
||||
t.Fatalf("creating test file failed: %s", err)
|
||||
}
|
||||
err = f.Close()
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure closing file: %v", err)
|
||||
}
|
||||
stat, err = os.Stat(path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure to Stat file: %v", err)
|
||||
}
|
||||
// Put a seemingly duplicate cookie in the jar to see if we can trigger an incorrect removal from the paths map
|
||||
err = port.AssociatePath(path, stat, FILE_MODIFIED, c)
|
||||
if err != nil {
|
||||
t.Errorf("unexpected failure associating file: %v", err)
|
||||
}
|
||||
port.checkInternals(t, 0, 1, 3, 2)
|
||||
|
||||
// run the garbage collector so that if we messed up it should be painfully clear
|
||||
runtime.GC()
|
||||
|
||||
// Before the fix, this would cause a nil pointer exception
|
||||
e, err := port.GetOne(nil)
|
||||
if err != nil {
|
||||
t.Errorf("failed to get an event: %v", err)
|
||||
}
|
||||
port.checkInternals(t, 0, 1, 2, 1)
|
||||
if e.Cookie != "cookie1" {
|
||||
t.Errorf(`expected "cookie1", got "%v"`, e.Cookie)
|
||||
}
|
||||
// Make sure that a cookie of the same value doesn't cause removal from the paths map incorrectly
|
||||
e, err = port.GetOne(nil)
|
||||
if err != nil {
|
||||
t.Errorf("failed to get an event: %v", err)
|
||||
}
|
||||
port.checkInternals(t, 0, 1, 1, 0)
|
||||
if e.Cookie != "cookie2" {
|
||||
t.Errorf(`expected "cookie2", got "%v"`, e.Cookie)
|
||||
}
|
||||
|
||||
err = os.Remove(path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected failure deleting file: %v", err)
|
||||
}
|
||||
// Event has fired, but until processed it should still be in the map
|
||||
port.checkInternals(t, 0, 1, 1, 1)
|
||||
e, err = port.GetOne(nil)
|
||||
if err != nil {
|
||||
t.Errorf("failed to get an event: %v", err)
|
||||
}
|
||||
if e.Cookie != "cookie2" {
|
||||
t.Errorf(`expected "cookie2", got "%v"`, e.Cookie)
|
||||
}
|
||||
// The maps should be empty and there should be no pending events
|
||||
port.checkInternals(t, 0, 0, 0, 0)
|
||||
}
|
||||
|
|
@ -737,8 +737,20 @@ type fileObjCookie struct {
|
|||
type EventPort struct {
|
||||
port int
|
||||
mu sync.Mutex
|
||||
fds map[uintptr]interface{}
|
||||
fds map[uintptr]*fileObjCookie
|
||||
paths map[string]*fileObjCookie
|
||||
// The user cookie presents an interesting challenge from a memory management perspective.
|
||||
// There are two paths by which we can discover that it is no longer in use:
|
||||
// 1. The user calls port_dissociate before any events fire
|
||||
// 2. An event fires and we return it to the user
|
||||
// The tricky situation is if the event has fired in the kernel but
|
||||
// the user hasn't requested/received it yet.
|
||||
// If the user wants to port_dissociate before the event has been processed,
|
||||
// we should handle things gracefully. To do so, we need to keep an extra
|
||||
// reference to the cookie around until the event is processed
|
||||
// thus the otherwise seemingly extraneous "cookies" map
|
||||
// The key of this map is a pointer to the corresponding &fCookie.cookie
|
||||
cookies map[*interface{}]*fileObjCookie
|
||||
}
|
||||
|
||||
// PortEvent is an abstraction of the port_event C struct.
|
||||
|
|
@ -762,9 +774,10 @@ func NewEventPort() (*EventPort, error) {
|
|||
return nil, err
|
||||
}
|
||||
e := &EventPort{
|
||||
port: port,
|
||||
fds: make(map[uintptr]interface{}),
|
||||
paths: make(map[string]*fileObjCookie),
|
||||
port: port,
|
||||
fds: make(map[uintptr]*fileObjCookie),
|
||||
paths: make(map[string]*fileObjCookie),
|
||||
cookies: make(map[*interface{}]*fileObjCookie),
|
||||
}
|
||||
return e, nil
|
||||
}
|
||||
|
|
@ -779,9 +792,13 @@ func NewEventPort() (*EventPort, error) {
|
|||
func (e *EventPort) Close() error {
|
||||
e.mu.Lock()
|
||||
defer e.mu.Unlock()
|
||||
err := Close(e.port)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
e.fds = nil
|
||||
e.paths = nil
|
||||
return Close(e.port)
|
||||
return nil
|
||||
}
|
||||
|
||||
// PathIsWatched checks to see if path is associated with this EventPort.
|
||||
|
|
@ -818,6 +835,7 @@ func (e *EventPort) AssociatePath(path string, stat os.FileInfo, events int, coo
|
|||
return err
|
||||
}
|
||||
e.paths[path] = fCookie
|
||||
e.cookies[&fCookie.cookie] = fCookie
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -830,11 +848,19 @@ func (e *EventPort) DissociatePath(path string) error {
|
|||
return fmt.Errorf("%v is not associated with this Event Port", path)
|
||||
}
|
||||
_, err := port_dissociate(e.port, PORT_SOURCE_FILE, uintptr(unsafe.Pointer(f.fobj)))
|
||||
if err != nil {
|
||||
// If the path is no longer associated with this event port (ENOENT)
|
||||
// we should delete it from our map. We can still return ENOENT to the caller.
|
||||
// But we need to save the cookie
|
||||
if err != nil && err != ENOENT {
|
||||
return err
|
||||
}
|
||||
if err == nil {
|
||||
// dissociate was successful, safe to delete the cookie
|
||||
fCookie := e.paths[path]
|
||||
delete(e.cookies, &fCookie.cookie)
|
||||
}
|
||||
delete(e.paths, path)
|
||||
return nil
|
||||
return err
|
||||
}
|
||||
|
||||
// AssociateFd wraps calls to port_associate(3c) on file descriptors.
|
||||
|
|
@ -844,12 +870,13 @@ func (e *EventPort) AssociateFd(fd uintptr, events int, cookie interface{}) erro
|
|||
if _, found := e.fds[fd]; found {
|
||||
return fmt.Errorf("%v is already associated with this Event Port", fd)
|
||||
}
|
||||
pcookie := &cookie
|
||||
_, err := port_associate(e.port, PORT_SOURCE_FD, fd, events, (*byte)(unsafe.Pointer(pcookie)))
|
||||
fCookie := &fileObjCookie{nil, cookie}
|
||||
_, err := port_associate(e.port, PORT_SOURCE_FD, fd, events, (*byte)(unsafe.Pointer(&fCookie.cookie)))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
e.fds[fd] = pcookie
|
||||
e.fds[fd] = fCookie
|
||||
e.cookies[&fCookie.cookie] = fCookie
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -862,11 +889,16 @@ func (e *EventPort) DissociateFd(fd uintptr) error {
|
|||
return fmt.Errorf("%v is not associated with this Event Port", fd)
|
||||
}
|
||||
_, err := port_dissociate(e.port, PORT_SOURCE_FD, fd)
|
||||
if err != nil {
|
||||
if err != nil && err != ENOENT {
|
||||
return err
|
||||
}
|
||||
if err == nil {
|
||||
// dissociate was successful, safe to delete the cookie
|
||||
fCookie := e.fds[fd]
|
||||
delete(e.cookies, &fCookie.cookie)
|
||||
}
|
||||
delete(e.fds, fd)
|
||||
return nil
|
||||
return err
|
||||
}
|
||||
|
||||
func createFileObj(name string, stat os.FileInfo) (*fileObj, error) {
|
||||
|
|
@ -894,26 +926,48 @@ func (e *EventPort) GetOne(t *Timespec) (*PortEvent, error) {
|
|||
return nil, err
|
||||
}
|
||||
p := new(PortEvent)
|
||||
p.Events = pe.Events
|
||||
p.Source = pe.Source
|
||||
e.mu.Lock()
|
||||
defer e.mu.Unlock()
|
||||
switch pe.Source {
|
||||
case PORT_SOURCE_FD:
|
||||
p.Fd = uintptr(pe.Object)
|
||||
cookie := (*interface{})(unsafe.Pointer(pe.User))
|
||||
p.Cookie = *cookie
|
||||
delete(e.fds, p.Fd)
|
||||
case PORT_SOURCE_FILE:
|
||||
p.fobj = (*fileObj)(unsafe.Pointer(uintptr(pe.Object)))
|
||||
p.Path = BytePtrToString((*byte)(unsafe.Pointer(p.fobj.Name)))
|
||||
cookie := (*interface{})(unsafe.Pointer(pe.User))
|
||||
p.Cookie = *cookie
|
||||
delete(e.paths, p.Path)
|
||||
}
|
||||
e.peIntToExt(pe, p)
|
||||
return p, nil
|
||||
}
|
||||
|
||||
// peIntToExt converts a cgo portEvent struct into the friendlier PortEvent
|
||||
// NOTE: Always call this function while holding the e.mu mutex
|
||||
func (e *EventPort) peIntToExt(peInt *portEvent, peExt *PortEvent) {
|
||||
peExt.Events = peInt.Events
|
||||
peExt.Source = peInt.Source
|
||||
cookie := (*interface{})(unsafe.Pointer(peInt.User))
|
||||
peExt.Cookie = *cookie
|
||||
switch peInt.Source {
|
||||
case PORT_SOURCE_FD:
|
||||
delete(e.cookies, cookie)
|
||||
peExt.Fd = uintptr(peInt.Object)
|
||||
// Only remove the fds entry if it exists and this cookie matches
|
||||
if fobj, ok := e.fds[peExt.Fd]; ok {
|
||||
if &fobj.cookie == cookie {
|
||||
delete(e.fds, peExt.Fd)
|
||||
}
|
||||
}
|
||||
case PORT_SOURCE_FILE:
|
||||
if fCookie, ok := e.cookies[cookie]; ok && uintptr(unsafe.Pointer(fCookie.fobj)) == uintptr(peInt.Object) {
|
||||
// Use our stashed reference rather than using unsafe on what we got back
|
||||
// the unsafe version would be (*fileObj)(unsafe.Pointer(uintptr(peInt.Object)))
|
||||
peExt.fobj = fCookie.fobj
|
||||
} else {
|
||||
panic("mismanaged memory")
|
||||
}
|
||||
delete(e.cookies, cookie)
|
||||
peExt.Path = BytePtrToString((*byte)(unsafe.Pointer(peExt.fobj.Name)))
|
||||
// Only remove the paths entry if it exists and this cookie matches
|
||||
if fobj, ok := e.paths[peExt.Path]; ok {
|
||||
if &fobj.cookie == cookie {
|
||||
delete(e.paths, peExt.Path)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Pending wraps port_getn(3c) and returns how many events are pending.
|
||||
func (e *EventPort) Pending() (int, error) {
|
||||
var n uint32 = 0
|
||||
|
|
@ -944,21 +998,7 @@ func (e *EventPort) Get(s []PortEvent, min int, timeout *Timespec) (int, error)
|
|||
e.mu.Lock()
|
||||
defer e.mu.Unlock()
|
||||
for i := 0; i < int(got); i++ {
|
||||
s[i].Events = ps[i].Events
|
||||
s[i].Source = ps[i].Source
|
||||
switch ps[i].Source {
|
||||
case PORT_SOURCE_FD:
|
||||
s[i].Fd = uintptr(ps[i].Object)
|
||||
cookie := (*interface{})(unsafe.Pointer(ps[i].User))
|
||||
s[i].Cookie = *cookie
|
||||
delete(e.fds, s[i].Fd)
|
||||
case PORT_SOURCE_FILE:
|
||||
s[i].fobj = (*fileObj)(unsafe.Pointer(uintptr(ps[i].Object)))
|
||||
s[i].Path = BytePtrToString((*byte)(unsafe.Pointer(s[i].fobj.Name)))
|
||||
cookie := (*interface{})(unsafe.Pointer(ps[i].User))
|
||||
s[i].Cookie = *cookie
|
||||
delete(e.paths, s[i].Path)
|
||||
}
|
||||
e.peIntToExt(&ps[i], &s[i])
|
||||
}
|
||||
return int(got), err
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче