Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/sys/unix: EpollEvent use void *ptr instead of int fd #45540

Open
pierre-emmanuelJ opened this issue Apr 13, 2021 · 7 comments
Open

x/sys/unix: EpollEvent use void *ptr instead of int fd #45540

pierre-emmanuelJ opened this issue Apr 13, 2021 · 7 comments

Comments

@pierre-emmanuelJ
Copy link

@pierre-emmanuelJ pierre-emmanuelJ commented Apr 13, 2021

What version of Go are you using (go version)?

go version go1.16.2 linux/amd64

I'm using EPOLL(7) from the sys/unix package, since epoll_event struct have a C union epoll_data

typedef union epoll_data {
    void    *ptr;
    int      fd;
    uint32_t u32;
    uint64_t u64;
} epoll_data_t;

struct epoll_event {
    uint32_t     events;    /* Epoll events */
    epoll_data_t data;      /* User data variable */
};

The sys/unix package supporting the struct EpollEvent with Pad
at the end of the struct to fill the union size.

type EpollEvent struct {
	Events uint32
	Fd     int32
	Pad    int32
}

The goal of this issue, is to use EpollEvent with void *ptr and not int fd

I created a struct epollEvent and called the syscall manually
To try to implement the change.

type epollEvent struct {
	Events uint32
	Ptr    uintptr
}

func epollWait(epfd int, events []epollEvent, msec int) (n int, err error) {
	var _p0 unsafe.Pointer
	if len(events) > 0 {
		_p0 = unsafe.Pointer(&events[0])
	} else {
		_p0 = unsafe.Pointer(&_zero)
	}

	r0, _, e1 := unix.Syscall6(unix.SYS_EPOLL_WAIT, uintptr(epfd), uintptr(_p0), uintptr(len(events)), uintptr(msec), 0, 0)
	n = int(r0)
	if e1 != 0 {
		err = errnoErr(e1)
	}

	return
}

The implementation issue

When I initialize my slice epollEvent and all Ptr uintpr to a struct instance associated to an epoll event.
And I call epollWait I retrieve my address associated to the event.

But the problem here is the Golang runtime, sometime the runtime move or GC the data associated to my address.
I tried to use unsafe.Pointer insteaf of uintptr but the issue is the same.

In this case what is the best way to create data and associate it to my Ptr
To be retrieved it later by epoll_wait(2) syscall?

Do you think the Idea I would like to realize it possible, if yes I could propose some change to support both in sys/unix package fdmode and ptr mode.

Thanks a lot

@tklauser tklauser changed the title sys/unix: EpollEvent use void *ptr instead of int fd x/sys/unix: EpollEvent use void *ptr instead of int fd Apr 13, 2021
@gopherbot gopherbot added this to the Unreleased milestone Apr 13, 2021
@tklauser
Copy link
Member

@tklauser tklauser commented Apr 13, 2021

This was previously proposed in #32192, which was merged into #32153. The decision there was to not change EpollEvent: #32153 (comment)

As per #32192 (comment) having an EpollEventPtr variant would mean that we'd have to introduce another EpollCtl variant. And the kernel might return a mix of the two types, which will probably become messy to deal with.

@pierre-emmanuelJ Could you elaborate a bit on your use case (e.g. with an example using your epollWait implementation), i.e. why you'd want to send pointers to the kernel?

/cc @ianlancetaylor @bradfitz

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 13, 2021

Would it make sense to expose the u32 and u64 fields of epoll_data instead?

That could be pretty straightforward to use in conjunction with runtime/cgo.Handle (#37033).

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 13, 2021

Yes, the new runtime/cgo.Handle is a good suggestion. Note that passing a Go pointer to epoll isn't safe: nothing will keep that pointer alive. In certain circumstances it's even possible for the pointer to move from Go's point of view, but the copy in epoll will not be updated.

@pierre-emmanuelJ
Copy link
Author

@pierre-emmanuelJ pierre-emmanuelJ commented Apr 14, 2021

A sample of the event package

package epoll

const (
	// MaxEvents represents the max events for an epoll instance.
	MaxEvents = 32
)

type Event struct {
	Fd     int32
	send   bool
	Cookie interface{}
}

// Send to the handler HandlerOut
func (e *Event) Send() {
	e.send = true
}

// Receive to the handler HandlerIn
func (e *Event) Receive() {
	e.send = false
}

// Handlers represents all handler for incoming events.
type Handlers struct {
	HandlerIn            func(*Event) bool
	HandlerOut           func(*Event) bool
	HandlerErr           func(*Event)
	HandlerNewConnection func(socket int) (int, interface{}, error)
}

// Epoll represents an epoll instance.
type Epoll struct {
	ev       *epollEvent
	epollfd  int
	fd       int
	handlers Handlers

	events [MaxEvents]epollEvent
}

// New returns an epoll instance.
func New(fd int, h Handlers) (*Epoll, error) {
	ptr := C.malloc(C.ulong(unsafe.Sizeof(Event{})))

	(*Event)(ptr).Fd = int32(fd)
	(*Event)(ptr).send = false
	(*Event)(ptr).Cookie = nil

	ev := &epollEvent{
		Ptr: ptr,
	}

	epfd, err := unix.EpollCreate1(0)
	if err != nil {
		return nil, err
	}

	ev.Events = unix.EPOLLIN

	if err := epollCtl(epfd, unix.EPOLL_CTL_ADD, fd, ev); err != nil {
		return nil, err
	}

	return &Epoll{
		ev:       ev,
		epollfd:  epfd,
		fd:       fd,
		handlers: h,
	}, nil
}

// Close epoll File Descriptor.
func (e *Epoll) Close() {
	unix.Close(e.epollfd)
}

func (e *Epoll) Wait() {
	for {
		nfds, err := e.wait()
		if err != nil {
			log.Fatal(err)
		}

		for i := 0; i < int(nfds); i++ {
			if e.incoming(i) {
				fd, data, err := e.handlers.HandlerNewConnection(e.fd)
				if err != nil {
					log.Println(err)
					continue
				}

				if err := e.add(fd, data); err != nil {
					log.Fatal(err)
				}

				continue
			}

			if err := e.executeHandlers(&e.events[i]); err != nil {
				log.Fatal(err)
			}
		}
	}
}

func (e *Epoll) wait() (int, error) {
	var nfds int
	var err error

	for {
		nfds, err = epollWait(e.epollfd, e.events[:], -1)
		if err == unix.EINTR {
			continue
		}

		break
	}
	if err != nil {
		return 0, err
	}

	return nfds, nil
}

func (e *Epoll) incoming(index int) bool {
	return (*(*Event)(e.events[index].Ptr)).Fd == int32(e.fd)
}

func (e *Epoll) add(fd int, data interface{}) error {
	err := unix.SetNonblock(fd, true)
	if err != nil {
		return err
	}

	e.ev.Events = unix.EPOLLIN

	ptr := C.malloc(C.ulong(unsafe.Sizeof(Event{})))
	(*Event)(ptr).Fd = int32(fd)
	(*Event)(ptr).send = false
	(*Event)(ptr).Cookie = data
	e.ev.Ptr = unsafe.Pointer(ptr)

	return epollCtl(e.epollfd, unix.EPOLL_CTL_ADD, fd, e.ev)
}

func (e *Epoll) executeHandlers(ev *epollEvent) error {
	event := (*Event)(ev.Ptr)

	if (ev.Events & (unix.EPOLLIN | unix.EPOLLERR | unix.EPOLLHUP)) != 0 {
		if !e.handlers.HandlerIn(event) {
			return nil
		}

		if event.send {
			if err := e.out(event.Fd); err != nil {
				return err
			}
		}

		return nil
	}

	if (ev.Events & (unix.EPOLLOUT | unix.EPOLLERR | unix.EPOLLHUP)) != 0 {
		if !e.handlers.HandlerOut(event) {
			C.free(ev.Ptr)
			return nil
		}

		if !event.send {
			if err := e.in(event.Fd); err != nil {
				return err
			}
		}

		return nil
	}

	if (ev.Events & (unix.EPOLLERR | unix.EPOLLHUP)) != 0 {
		e.handlers.HandlerErr(event)
		C.free(ev.Ptr)
		return nil
	}

	log.Println("Unknown Epoll event")

	return nil
}

func (e *Epoll) out(fd int32) error {
	event := (*Event)(e.ev.Ptr)

	event.Fd = fd
	e.ev.Events = unix.EPOLLOUT

	return epollCtl(e.epollfd, unix.EPOLL_CTL_MOD, int(fd), e.ev)
}

func (e *Epoll) in(fd int32) error {
	event := (*Event)(e.ev.Ptr)

	event.Fd = fd
	e.ev.Events = unix.EPOLLIN

	return epollCtl(e.epollfd, unix.EPOLL_CTL_MOD, int(fd), e.ev)
}

My file private.go to upstream the unix package:

package epoll

import (
	"syscall"
	"unsafe"

	"golang.org/x/sys/unix"
)

var (
	_zero uintptr
)

type epollEvent struct {
	Events uint32
	Ptr    unsafe.Pointer
}

// errnoErr returns common boxed Errno values, to prevent
// allocations at runtime.
func errnoErr(e syscall.Errno) error {
	switch e {
	case 0:
		return nil
	case unix.EAGAIN:
		return syscall.EAGAIN
	case unix.EINVAL:
		return syscall.EINVAL
	case unix.ENOENT:
		return syscall.ENOENT
	}
	return e
}

func epollWait(epfd int, events []epollEvent, msec int) (n int, err error) {
	var _p0 unsafe.Pointer
	if len(events) > 0 {
		_p0 = unsafe.Pointer(&events[0])
	} else {
		_p0 = unsafe.Pointer(&_zero)
	}

	r0, _, e1 := unix.Syscall6(unix.SYS_EPOLL_WAIT, uintptr(epfd), uintptr(_p0), uintptr(len(events)), uintptr(msec), 0, 0)
	n = int(r0)
	if e1 != 0 {
		err = errnoErr(e1)
	}

	return
}

func epollCtl(epfd int, op int, fd int, event *epollEvent) (err error) {
	_, _, e1 := unix.RawSyscall6(unix.SYS_EPOLL_CTL, uintptr(epfd), uintptr(op), uintptr(fd), uintptr(unsafe.Pointer(event)), 0, 0)
	if e1 != 0 {
		err = errnoErr(e1)
	}

	return
}

Example of usage:

	socket, err := newSocket(ip, port)
	if err != nil {
		log.Fatal(err)
	}
	defer unix.Close(socket)

	h := epoll.Handlers{
		HandlerIn:  func(e *epoll.Event) {
			// in stuff
		},
		HandlerOut: func(e *epoll.Event) {
			// out stuff
		},
		HandlerErr: func(e *epoll.Event) {
			// err stuff
		},
		HandlerNewConnection: func(fd int) (int, interface{}, error) {
			// call from CGO
			conn, err := C.GotNewConnection(fd, config)
			if err != nil {
				return 0, nil, err
			}

			return conn.FD, conn, nil
		},
	}

	epoll, err := epoll.New(socket, h)
	if err != nil {
		log.Fatal(err)
	}
	defer epoll.Close()

	epoll.Wait()

In this sample of code I tried to use pointers from MALLOC(3) instead of Go pointers. to prevent this issue.
(malloc value returns are not checked)

@pierre-emmanuelJ
Copy link
Author

@pierre-emmanuelJ pierre-emmanuelJ commented Apr 14, 2021

It's very weird @tklauser after calling epollWait with my events slice, ptr is populated with pointers I have not registered in, and that cause segfault so.

A thing I can not understand because here I register the pointer related to new event:

e.ev.Events = unix.EPOLLIN

ptr := C.malloc(C.ulong(unsafe.Sizeof(Event{})))
(*Event)(ptr).Fd = int32(fd)
(*Event)(ptr).send = false
(*Event)(ptr).Cookie = data
e.ev.Ptr = unsafe.Pointer(ptr)

return epollCtl(e.epollfd, unix.EPOLL_CTL_ADD, fd, e.ev)

After debugging pointer address I registered before epollCtl with ADD option and after. I can retrieve my initial pointer I'm using to register new connection. But pointer added after an incoming connection are not retrieved by epollWait. epollWait returns me totally unknown pointer.

@pierre-emmanuelJ
Copy link
Author

@pierre-emmanuelJ pierre-emmanuelJ commented Apr 15, 2021

Here is a more compact example if you want (quick implementation):
https://gist.github.com/pierre-emmanuelJ/4562322b24020e24ca4389b6aa7f7d3d

Same here epollWait populate me the events slice with unknown pointer in unsafe.pointer Ptr.
Did I miss something in the unix.Syscall6 call?

@pierre-emmanuelJ
Copy link
Author

@pierre-emmanuelJ pierre-emmanuelJ commented Apr 20, 2021

@tklauser I found the main reason of the issue mentioned in those examples, if you want to use epoll in ptr mode correctly with the new runtime/cgo.Handlestuff or malloc there are some points to mention about memory alignment.

Since the C struct epoll_event is using attribute packed:

#define __EPOLL_PACKED __attribute__ ((__packed__))

struct epoll_event
{
  uint32_t events;	/* Epoll events */
  epoll_data_t data;	/* User data variable */
} __EPOLL_PACKED;

the sizeof the struct is 12 bytes. In Golang it's not possible to use attribute packed.
This following structure could not be used with epoll:

type EpollEvent struct {
	Events uint32
	Ptr    unsafe.Pointer // Or uintptr
}

The sizeof this struct is 16 bytes.

To properly use epoll in ptr mode the solution is to use CGO to instantiate an epoll_event struct or use something like this:

type EpollEvent struct {
	Events uint32
	Ptr    [8]byte
}

And then use the binary package to retrieve the pointer address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants