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.

Sign up for GitHub

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

Jump to bottom

Fix windows fd #25

Merged
merged 6 commits into from
Oct 3, 2020
Merged

Fix windows fd #25

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d3d38a4
main: fix windows file descriptors #20
johnjuuljensen Sep 29, 2020
c039df9
main: fix epoll by reverting some changes
sreimers Sep 29, 2020
399b2f6
main: fix zero file handler and descriptor detection
sreimers Sep 29, 2020
11c2ffd
main: cleanup
sreimers Sep 29, 2020
de3f2ff
main: fix file descriptor index win32
sreimers Sep 29, 2020
fe0d21f
main: fix to early fd lookup (first fd_listen requires poll_setup)
sreimers Sep 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 75 additions & 24 deletions src/main/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,17 @@ enum {
#endif
};

/** File descriptor handler struct */
struct fhs {
int fd; /**< File Descriptor */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int fd is only used on windows ?

perhaps change the type of fd to SOCKET to make it clear that it is a windows type:

typedef UINT_PTR        SOCKET;

it will also let the compiler warn about wrong api usage etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As i can see SOCKET is casted to int through SOK_CAST like here:
https://github.com/baresip/re/blob/master/src/tcp/tcp.c#L510

I can make re->fhs[i].fd WIN32 only, with some extra ifdefs

int flags; /**< Polling flags (Read, Write, etc.) */
fd_h* fh; /**< Event handler */
void* arg; /**< Handler argument */
};

/** Polling loop data */
struct re {
/** File descriptor handler set */
struct {
int flags; /**< Polling flags (Read, Write, etc.) */
fd_h *fh; /**< Event handler */
void *arg; /**< Handler argument */
} *fhs;
struct fhs *fhs; /** File descriptor handler set */
int maxfds; /**< Maximum number of polling fds */
int nfds; /**< Number of active file descriptors */
enum poll_method method; /**< The current polling method */
Expand Down Expand Up @@ -221,30 +223,62 @@ static struct re *re_get(void)

#endif

#ifdef WIN32
/**
* This code emulates POSIX numbering. There is no locking,
* so zero thread-safety.
*
* @param re Poll state
* @param fd File descriptor
*
* @return fhs index if success, otherwise -1
*/
static int lookup_fd_index(struct re* re, int fd) {
int i;

for (i = 0; i < re->nfds; i++) {
if (!re->fhs[i].fh)
continue;

if (re->fhs[i].fd == fd)
return i;
}

/* if nothing is found a linear search for the first
* zeroed handler */
for (i = 0; i < re->maxfds; i++) {
if (!re->fhs[i].fh)
return i;
}

return -1;
}
#endif

#if MAIN_DEBUG
/**
* Call the application event handler
*
* @param re Poll state
* @param fd File descriptor
* @param i File descriptor handler index
* @param flags Event flags
*/
static void fd_handler(struct re *re, int fd, int flags)
static void fd_handler(struct re *re, int i, int flags)
{
const uint64_t tick = tmr_jiffies();
uint32_t diff;

DEBUG_INFO("event on fd=%d (flags=0x%02x)...\n", fd, flags);
DEBUG_INFO("event on fd=%d index=%d (flags=0x%02x)...\n",
re->fhs[i].fd, i, flags);

re->fhs[fd].fh(flags, re->fhs[fd].arg);
re->fhs[i].fh(flags, re->fhs[i].arg);

diff = (uint32_t)(tmr_jiffies() - tick);

if (diff > MAX_BLOCKING) {
DEBUG_WARNING("long async blocking: %u>%u ms (h=%p arg=%p)\n",
diff, MAX_BLOCKING,
re->fhs[fd].fh, re->fhs[fd].arg);
re->fhs[i].fh, re->fhs[i].arg);
}
}
#endif
Expand Down Expand Up @@ -565,6 +599,7 @@ int fd_listen(int fd, int flags, fd_h *fh, void *arg)
{
struct re *re = re_get();
int err = 0;
int i = fd;

DEBUG_INFO("fd_listen: fd=%d flags=0x%02x\n", fd, flags);

Expand All @@ -579,7 +614,16 @@ int fd_listen(int fd, int flags, fd_h *fh, void *arg)
return err;
}

if (fd >= re->maxfds) {
#ifdef WIN32
/* Windows file descriptors do not follow POSIX standard ranges. */
i = lookup_fd_index(re, fd);
if (i < 0) {
DEBUG_WARNING("fd_listen: fd=%d - no free fd_index\n", fd);
return EMFILE;
}
#endif

if (i >= re->maxfds) {
if (flags) {
DEBUG_WARNING("fd_listen: fd=%d flags=0x%02x"
" - Max %d fds\n",
Expand All @@ -590,12 +634,13 @@ int fd_listen(int fd, int flags, fd_h *fh, void *arg)

/* Update fh set */
if (re->fhs) {
re->fhs[fd].flags = flags;
re->fhs[fd].fh = fh;
re->fhs[fd].arg = arg;
re->fhs[i].fd = fd;
re->fhs[i].flags = flags;
re->fhs[i].fh = fh;
re->fhs[i].arg = arg;
}

re->nfds = max(re->nfds, fd+1);
re->nfds = max(re->nfds, i+1);

switch (re->method) {

Expand Down Expand Up @@ -656,7 +701,7 @@ void fd_close(int fd)
static int fd_poll(struct re *re)
{
const uint64_t to = tmr_next_timeout(&re->tmrl);
int i, n;
int i, n, index;
#ifdef HAVE_SELECT
fd_set rfds, wfds, efds;
#endif
Expand All @@ -683,15 +728,16 @@ static int fd_poll(struct re *re)
FD_ZERO(&efds);

for (i=0; i<re->nfds; i++) {
int fd = re->fhs[i].fd;
if (!re->fhs[i].fh)
continue;

if (re->fhs[i].flags & FD_READ)
FD_SET(i, &rfds);
FD_SET(fd, &rfds);
if (re->fhs[i].flags & FD_WRITE)
FD_SET(i, &wfds);
FD_SET(fd, &wfds);
if (re->fhs[i].flags & FD_EXCEPT)
FD_SET(i, &efds);
FD_SET(fd, &efds);
}

#ifdef WIN32
Expand Down Expand Up @@ -767,7 +813,7 @@ static int fd_poll(struct re *re)
#endif
#ifdef HAVE_SELECT
case METHOD_SELECT:
fd = i;
fd = re->fhs[i].fd;
if (FD_ISSET(fd, &rfds))
flags |= FD_READ;
if (FD_ISSET(fd, &wfds))
Expand Down Expand Up @@ -837,12 +883,17 @@ static int fd_poll(struct re *re)

if (!flags)
continue;
#ifdef WIN32
index = i;
#else
index = fd;
#endif

if (re->fhs[fd].fh) {
if (re->fhs[index].fh) {
#if MAIN_DEBUG
fd_handler(re, fd, flags);
fd_handler(re, index, flags);
#else
re->fhs[fd].fh(flags, re->fhs[fd].arg);
re->fhs[index].fh(flags, re->fhs[index].arg);
#endif
}

Expand Down