Page MenuHomePhabricator

Lack of G_IO_ERR condition for network sockets in glib integration
Closed, ResolvedPublic

Description

Glib integration with using of select() syscall doesn't properly propagates G_IO_ERR condition for network sockets. Problem relies in _ecore_glib_context_poll_to() where rewriting filedescriptor events to GPollFD structures reside. Invalid code:

if (FD_ISSET(itr->fd, efds) && (itr->events & (G_IO_HUP | G_IO_ERR)))
  {
     itr->revents |= G_IO_ERR;
     ready--;
  }

Problem relies in efds are exception filedescriptors, what really means they are set for out-of-band data (urgent data for example). This code likely should set itr->revents |= G_IO_PRI.

More problematic is G_IO_ERR condition. Detection can be done when write condition (G_IO_OUT) occurs. Proposed patch is in the attachement (+test code).

There's define ECORELOOP on test code beginning. If not defined glib loop is used otherwise ecore loop. Run the program:
./test 127.0.0.1 12345
Assuming that port 12345 is not opened RST segment will be sent. This should immediately finish with G_IO_ERR, but for ecore loop it finishes with G_IO_OUT what is wrong.

Problem with this patch is that for all filedescriptors stat() is and if fd is socket getpeername() syscall are called. This can slightly increase load(), I didn't performed such tests.

dt9 created this task.Jul 13 2017, 3:06 AM
dt9 updated the task description. (Show Details)Jul 13 2017, 3:09 AM
dt9 updated the task description. (Show Details)Jul 13 2017, 3:14 AM
bu5hm4n assigned this task to q66.Jun 11 2018, 2:08 AM
bu5hm4n triaged this task as Normal priority.
bu5hm4n added a subscriber: bu5hm4n.

Thank you for the patch!

q66 reassigned this task from q66 to raster.Jun 11 2018, 2:51 AM
q66 added a subscriber: q66.

raster can probably provide better feedback on this

zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:57 AM
zmike edited projects, added efl: networking; removed Restricted Project.Jun 11 2018, 8:47 AM

well the plus - it's only on the glib integration path... so it only affects glib things. :)