[vlc-devel] [PATCH 1/3] [RFC] poll: keep the WSAEVENT per socket in memory while the socket is alive
Steve Lhomme
robux4 at gmail.com
Sat Jul 16 13:51:01 CEST 2016
On Sat, Jul 16, 2016 at 11:38 AM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le keskiviikkona 13. heinäkuuta 2016, 18.50.20 EEST Steve Lhomme a écrit :
>> In some cases we lose some events because the event is destroyed and
>> recreated only when we need to poll. It's better to keep the event receiver
>> so we don't miss events after the first WSAEnumNetworkEvents() call.
>> ---
>> compat/Makefile.am | 1 +
>> compat/poll.c | 22 ++++++++++++++++++++--
>> compat/poll.h | 28 ++++++++++++++++++++++++++++
>> include/vlc_network.h | 3 ++-
>> src/libvlccore.sym | 1 +
>> src/missing.c | 9 +++++++++
>> src/win32/winsock.c | 8 ++++++++
>> 7 files changed, 69 insertions(+), 3 deletions(-)
>> create mode 100644 compat/poll.h
>>
>> diff --git a/compat/Makefile.am b/compat/Makefile.am
>> index 58094d4..e1f0c1d 100644
>> --- a/compat/Makefile.am
>> +++ b/compat/Makefile.am
>> @@ -2,6 +2,7 @@ pkglib_LTLIBRARIES = libcompat.la
>> libcompat_la_SOURCES = dummy.c
>> libcompat_la_LIBADD = $(LTLIBOBJS) $(LIBRT)
>> libcompat_la_LDFLAGS = -no-undefined -static
>> +noinst_HEADERS = poll.h
>>
>> BUILT_SOURCES = dummy.c
>> CLEANFILES = dummy.c
>> diff --git a/compat/poll.c b/compat/poll.c
>> index fa94c93..91c2d81 100644
>> --- a/compat/poll.c
>> +++ b/compat/poll.c
>> @@ -108,6 +108,25 @@ int (poll) (struct pollfd *fds, unsigned nfds, int
>> timeout) #else
>> # include <windows.h>
>> # include <winsock2.h>
>> +# include "poll.h"
>> +
>> +static WSAEVENT opened_events[4096] = {};
>
> Does Windows actually warranty that socket handles are "small" numbers? It
> would work if we wrapped sockets behind _open_osfhandle(), but we do not (and
> it would bring some other complications).
Not sure. This patch is actually not the one I intended to send
although the idea is the same. Now I use realloc() on a growing array.
I'm still not 100% happy with what I have at the moment as it doesn't
seem to work all the time. A solution I proposed a while ago just
using select() with timeout and a dummy socket works 100% of the time.
>> +
>> +WSAEVENT *win32_get_socket_event(int fd)
>> +{
>> + if (opened_events[fd] == 0)
>> + opened_events[fd] = WSACreateEvent();
>> + return opened_events[fd];
>> +}
>> +
>> +void win32_close_socket_event(int fd)
>> +{
>> + if (opened_events[fd] != 0)
>> + {
>> + WSACloseEvent(opened_events[fd]);
>> + opened_events[fd] = 0;
>> + }
>> +}
>
> poll() is supposed to be reentrant. For instance, one thread can wait for
> reading while another thread waits for writing on the same socket.
Do we have this case in VLC ? That a reading and writing socket is
shared among threads ? I thought about that, but it makes it more
complex to use a static mutex from compat.
> I also suspect this does not handle duplicated socket handles correctly.
The same socket id can be used for 2 different sockets ? That sounds odd.
>> int poll(struct pollfd *fds, unsigned nfds, int timeout)
>> {
>> @@ -154,7 +173,7 @@ int poll(struct pollfd *fds, unsigned nfds, int timeout)
>>
>> fds[i].revents = 0;
>>
>> - evts[i] = WSACreateEvent();
>> + evts[i] = win32_get_socket_event(fd);
>> if (evts[i] == WSA_INVALID_EVENT)
>> {
>> while (i > 0)
>> @@ -201,7 +220,6 @@ int poll(struct pollfd *fds, unsigned nfds, int timeout)
>> if (WSAEnumNetworkEvents(fds[i].fd, evts[i], &ne))
>> memset(&ne, 0, sizeof (ne));
>> WSAEventSelect(fds[i].fd, evts[i], 0);
>> - WSACloseEvent(evts[i]);
>>
>> if (ne.lNetworkEvents & FD_CONNECT)
>> {
>> diff --git a/compat/poll.h b/compat/poll.h
>> new file mode 100644
>> index 0000000..78ffc8c
>> --- /dev/null
>> +++ b/compat/poll.h
>> @@ -0,0 +1,28 @@
>> +/**************************************************************************
>> *** + * poll.c: poll() emulation
>> +
>> ***************************************************************************
>> ** + * Copyright © 2016 VLC authors, VideoLAN and VideoLabs
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU Lesser General Public License as published by
>> + * the Free Software Foundation; either version 2.1 of the License, or + *
>> (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public License
>> + * along with this program; if not, write to the Free Software Foundation,
>> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA. +
>> ***************************************************************************
>> **/ +
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +
>> +#ifdef _WIN32
>> +extern WSAEVENT *win32_get_socket_event(int fd);
>> +extern void win32_close_socket_event(int fd);
>> +#endif
>> diff --git a/include/vlc_network.h b/include/vlc_network.h
>> index 8779ad2..1b627eb 100644
>> --- a/include/vlc_network.h
>> +++ b/include/vlc_network.h
>> @@ -133,7 +133,7 @@ VLC_API ssize_t net_vaPrintf( vlc_object_t *p_this, int
>> fd, const char *psz_fmt, # define SHUT_RD SD_RECEIVE
>> # define SHUT_WR SD_SEND
>> # define SHUT_RDWR SD_BOTH
>> -# define net_Close( fd ) closesocket ((SOCKET)fd)
>> +# define net_Close( fd ) vlc_net_close (fd)
>> #else
>> # ifdef __OS2__
>> # define SHUT_RD 0
>> @@ -144,6 +144,7 @@ VLC_API ssize_t net_vaPrintf( vlc_object_t *p_this, int
>> fd, const char *psz_fmt, VLC_API int vlc_close(int);
>> # define net_Close( fd ) (void)vlc_close (fd)
>> #endif
>> +VLC_API int vlc_net_close( int fd );
>>
>> /* Portable network names/addresses resolution layer */
>>
>> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
>> index 89e74f8..a1211f1 100644
>> --- a/src/libvlccore.sym
>> +++ b/src/libvlccore.sym
>> @@ -290,6 +290,7 @@ net_Read
>> net_SetCSCov
>> net_vaPrintf
>> net_Write
>> +vlc_net_close
>> NTPtime64
>> picture_BlendSubpicture
>> picture_CopyPixels
>> diff --git a/src/missing.c b/src/missing.c
>> index 692b9b5..e533dc3 100644
>> --- a/src/missing.c
>> +++ b/src/missing.c
>> @@ -38,6 +38,15 @@
>> #include <vlc_common.h>
>> #include <assert.h>
>>
>> +#ifndef _WIN32
>> +# include <vlc_network.h>
>> +int vlc_net_close( int fd )
>> +{
>> + (void) fd;
>> + vlc_assert_unreachable ();
>> +}
>> +#endif
>> +
>> #ifndef ENABLE_HTTPD
>> # include <vlc_httpd.h>
>>
>> diff --git a/src/win32/winsock.c b/src/win32/winsock.c
>> index cac83fd..185303d 100644
>> --- a/src/win32/winsock.c
>> +++ b/src/win32/winsock.c
>> @@ -25,6 +25,14 @@
>> #include <vlc_common.h>
>> #include <vlc_network.h>
>>
>> +#include "../../compat/poll.h"
>> +
>> +int vlc_net_close( int fd )
>> +{
>> + win32_close_socket_event(fd);
>> + return closesocket((SOCKET)fd);
>> +}
>> +
>> #if 0
>> ssize_t vlc_sendmsg (int s, struct msghdr *hdr, int flags)
>> {
>
>
> --
> Rémi Denis-Courmont
> http://www.remlab.net/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list