Project

General

Profile

Bug #8634

epoll fails to wake on certain edge-triggered conditions

Added by Patrick Mooney over 3 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Category:
kernel
Start date:
2017-09-07
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

This replicates downstream bug OS-5882

There are certain circumstances under which epoll will fail to wake and emit events for descriptors under edge-triggered polling.

This small test program completes successfully on Linux but hangs on SmartOS:

#include <stdio.h>
#include <unistd.h>
#include <pthread.h>
#include <sys/eventfd.h>
#include <sys/epoll.h>

int evfd = -1;

void *writer(void *args) {
        uint64_t val = 1;

        usleep(1000);
        write(evfd, &val, sizeof (val));
    return NULL;
}

int main() {
        pthread_t thread_write;
        struct epoll_event epev;
        int epfd;

        evfd = eventfd(0, EFD_CLOEXEC|EFD_NONBLOCK);
        epfd = epoll_create1(0);

        epev.events = EPOLLIN|EPOLLOUT|EPOLLET;
        epev.data.fd = evfd;
        epoll_ctl(epfd, EPOLL_CTL_ADD, evfd, &epev);

        /* The eventfd is writable */
        epoll_wait(epfd, &epev, 1, -1);

        pthread_create(&thread_write, NULL, writer, NULL);

        /* The eventfd should report as readable once the above write succeeds */
        epoll_wait(epfd, &epev, 1, -1);

        pthread_join(thread_write, NULL);
}

One could argue that the same edge-triggered behavior in /dev/poll is also incorrect. Given the lack of proper specification, it's unclear.

#1

Updated by Patrick Mooney over 3 years ago

To test this, I compared results from excerpts of an internally-developed epoll test suite which covers the mentioned functionality.

#include <stdio.h>
#include <unistd.h>
#include <pthread.h>
#include <sys/eventfd.h>
#include <sys/epoll.h>

#include "common.h" 

int evfd = -1;
int ready = 0;
pthread_mutex_t lock;

void *
writer_func(void *args) {
        uint64_t val = 1;

        ready = 1;
        pthread_mutex_lock(&lock);
        pthread_mutex_unlock(&lock);

        /*
         * Allow the main thread time to enter the epoll_ctl.  Strictly speaking,
         * this does have the potential to race.  There doesn't appear to be a
         * convenient way to ensure proper ordering here.  The sleep should be
         * adequate for all but the most pathological cases.
         */
        usleep(1000);

        write(evfd, &val, sizeof (val));
        return (NULL);
}

int
main(int argc, char *argv[])
{
        pthread_t thread_write;
        struct epoll_event ev;
        int epfd;

        test_init(argc, argv);

        if ((evfd = eventfd(0, EFD_CLOEXEC|EFD_NONBLOCK)) < 0) {
                test_fatal("eventfd()");
        }
        if ((epfd = epoll_create1(0)) < 0) {
                test_fatal("epoll_create1()");
        }

        ev.events = EPOLLIN|EPOLLOUT|EPOLLET;
        ev.data.fd = evfd;
        if (epoll_ctl(epfd, EPOLL_CTL_ADD, evfd, &ev) < 0) {
                test_fatal("epoll_ctl(EPOLL_CTL_ADD)");
        }

        /* The eventfd is writable */
        test_equal(epoll_wait(epfd, &ev, 1, 0), 1);
        test_equal(ev.events, EPOLLOUT);
        test_equal(ev.data.fd, evfd);

        /* ET should yield no event now */
        test_equal(epoll_wait(epfd, &ev, 1, 0), 0);

        if (pthread_mutex_init(&lock, NULL) != 0) {
                test_fatal("pthread_mutex_init");
        }
        pthread_mutex_lock(&lock);
        pthread_create(&thread_write, NULL, writer_func, NULL);

        while (!ready) {
                usleep(1000);
        }
        pthread_mutex_unlock(&lock);

        /* The eventfd should report as readable once the above write succeeds */
        test_equal(epoll_wait(epfd, &ev, 1, 1000), 1);
        test_equal(ev.events, EPOLLIN|EPOLLOUT);
        test_equal(ev.data.fd, evfd);

        pthread_join(thread_write, NULL);
        test_done();
}

The updated code passes the test, unlike an unpatched -gate build.

#2

Updated by Electric Monk over 3 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 80d5689f5d4588adc071138e25e9d0d5252d9b55

commit  80d5689f5d4588adc071138e25e9d0d5252d9b55
Author: Patrick Mooney <pmooney@pfmooney.com>
Date:   2017-10-19T02:47:16.000Z

    8634 epoll fails to wake on certain edge-triggered conditions
    8635 epoll should not emit POLLNVAL
    8636 recursive epoll should emit EPOLLRDNORM
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Igor Kozhukhov <igor@dilos.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF