Re: [PATCH] Fix two bugs in the limit of large numbers of sockets:

[Erik Bray]

On Nov 7, 2017 16:36, "Corinna Vinschen" wrote:

Erik,

On Nov  7 16:11, Corinna Vinschen wrote:
> On Nov  7 14:44, Erik M. Bray wrote:
> > * Fix the maximum number of sockets allowed in the session to 2048,
> >   instead of making it relative to sizeof(wsa_event).
> >
> >   The original choice of 2048 was in order to fit the wsa_events array
> >   in the .cygwin_dll_common shared section, but there is still enough
> >   room to grow there to have 2048 sockets on 64-bit as well.
> >
> > * Return an error and set errno=ENOBUF if a socket can't be created
> >   due to this limit being reached.
> > ---
> >  winsup/cygwin/fhandler_socket.cc | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_
socket.cc
> > index 7a6dbdc..b8eda57 100644
> > --- a/winsup/cygwin/fhandler_socket.cc
> > +++ b/winsup/cygwin/fhandler_socket.cc
> > @@ -496,7 +496,7 @@ fhandler_socket::af_local_set_secret (char *buf)
> >  /* Maximum number of concurrently opened sockets from all Cygwin
processes
> > per session.  Note that shared sockets (through dup/fork/exec) are
> > counted as one socket. */
> > -#define NUM_SOCKS   (32768 / sizeof (wsa_event))
> > +#define NUM_SOCKS   ((unsigned int) 2048)
> >
> >  #define LOCK_EVENTS\
> >if (wsock_mtx && \
> > @@ -623,7 +623,14 @@ fhandler_socket::init_events ()
> >NtClose (wsock_mtx);
> >return false;
> >  }
> > -  wsock_events = search_wsa_event_slot (new_serial_number);
> > +  if (!(wsock_events = search_wsa_event_slot (new_serial_number)));
  ^^^
did you actually test this?

https://sourceware.org/git/?p=newlib-cygwin.git;a=commitdiff;h=
2e989b212955665384bf61ee82dd487844a9371a


I mean, of course. I'm running a build right now with this fix (and have
since built on my branch with no problem). Maybe I somehow fat-fingered a
semicolon before committing, sorry.

Re: [PATCH] Fix two bugs in the limit of large numbers of sockets:

[Corinna Vinschen]

Erik,

On Nov  7 16:11, Corinna Vinschen wrote:
> On Nov  7 14:44, Erik M. Bray wrote:
> > * Fix the maximum number of sockets allowed in the session to 2048,
> >   instead of making it relative to sizeof(wsa_event).
> > 
> >   The original choice of 2048 was in order to fit the wsa_events array
> >   in the .cygwin_dll_common shared section, but there is still enough
> >   room to grow there to have 2048 sockets on 64-bit as well.
> > 
> > * Return an error and set errno=ENOBUF if a socket can't be created
> >   due to this limit being reached.
> > ---
> >  winsup/cygwin/fhandler_socket.cc | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/winsup/cygwin/fhandler_socket.cc 
> > b/winsup/cygwin/fhandler_socket.cc
> > index 7a6dbdc..b8eda57 100644
> > --- a/winsup/cygwin/fhandler_socket.cc
> > +++ b/winsup/cygwin/fhandler_socket.cc
> > @@ -496,7 +496,7 @@ fhandler_socket::af_local_set_secret (char *buf)
> >  /* Maximum number of concurrently opened sockets from all Cygwin processes
> > per session.  Note that shared sockets (through dup/fork/exec) are
> > counted as one socket. */
> > -#define NUM_SOCKS   (32768 / sizeof (wsa_event))
> > +#define NUM_SOCKS   ((unsigned int) 2048)
> >  
> >  #define LOCK_EVENTS\
> >if (wsock_mtx && \
> > @@ -623,7 +623,14 @@ fhandler_socket::init_events ()
> >NtClose (wsock_mtx);
> >return false;
> >  }
> > -  wsock_events = search_wsa_event_slot (new_serial_number);
> > +  if (!(wsock_events = search_wsa_event_slot (new_serial_number)));
  ^^^
did you actually test this?

https://sourceware.org/git/?p=newlib-cygwin.git;a=commitdiff;h=2e989b212955665384bf61ee82dd487844a9371a


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat


signature.asc
Description: PGP signature

Re: [PATCH] Fix two bugs in the limit of large numbers of sockets:

[Jon Turney]

On 07/11/2017 13:44, Erik M. Bray wrote:

* Fix the maximum number of sockets allowed in the session to 2048,
   instead of making it relative to sizeof(wsa_event).

   The original choice of 2048 was in order to fit the wsa_events array
   in the .cygwin_dll_common shared section, but there is still enough
   room to grow there to have 2048 sockets on 64-bit as well.

* Return an error and set errno=ENOBUF if a socket can't be created
   due to this limit being reached.
---
  winsup/cygwin/fhandler_socket.cc | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc
index 7a6dbdc..b8eda57 100644
--- a/winsup/cygwin/fhandler_socket.cc
+++ b/winsup/cygwin/fhandler_socket.cc
@@ -496,7 +496,7 @@ fhandler_socket::af_local_set_secret (char *buf)
  /* Maximum number of concurrently opened sockets from all Cygwin processes
 per session.  Note that shared sockets (through dup/fork/exec) are
 counted as one socket. */
-#define NUM_SOCKS   (32768 / sizeof (wsa_event))
+#define NUM_SOCKS   ((unsigned int) 2048)
  
  #define LOCK_EVENTS	\

if (wsock_mtx && \
@@ -623,7 +623,14 @@ fhandler_socket::init_events ()
NtClose (wsock_mtx);
return false;
  }
-  wsock_events = search_wsa_event_slot (new_serial_number);
+  if (!(wsock_events = search_wsa_event_slot (new_serial_number)));


This has a stray ';' at the end of the line

../../../../winsup/cygwin/fhandler_socket.cc: In member function 'bool 
fhandler_socket::init_events()':
../../../../winsup/cygwin/fhandler_socket.cc:626:3: error: this 'if' 
clause does not guard... [-Werror=misleading-indentation]

   if (!(wsock_events = search_wsa_event_slot (new_serial_number)));
   ^~
../../../../winsup/cygwin/fhandler_socket.cc:627:5: note: ...this 
statement, but the latter is misleadingly indented as if it is guarded 
by the 'if'

 {
 ^


+{
+  set_errno (ENOBUFS);
+  NtClose (wsock_evt);
+  NtClose (wsock_mtx);
+  return false;
+}
+
/* sock type not yet set here. */
if (pc.dev == FH_UDP || pc.dev == FH_DGRAM)
  wsock_events->events = FD_WRITE;



How did you test this?

Re: [PATCH] Fix two bugs in the limit of large numbers of sockets:

[Corinna Vinschen]

On Nov  7 14:44, Erik M. Bray wrote:
> * Fix the maximum number of sockets allowed in the session to 2048,
>   instead of making it relative to sizeof(wsa_event).
> 
>   The original choice of 2048 was in order to fit the wsa_events array
>   in the .cygwin_dll_common shared section, but there is still enough
>   room to grow there to have 2048 sockets on 64-bit as well.
> 
> * Return an error and set errno=ENOBUF if a socket can't be created
>   due to this limit being reached.
> ---
>  winsup/cygwin/fhandler_socket.cc | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler_socket.cc 
> b/winsup/cygwin/fhandler_socket.cc
> index 7a6dbdc..b8eda57 100644
> --- a/winsup/cygwin/fhandler_socket.cc
> +++ b/winsup/cygwin/fhandler_socket.cc
> @@ -496,7 +496,7 @@ fhandler_socket::af_local_set_secret (char *buf)
>  /* Maximum number of concurrently opened sockets from all Cygwin processes
> per session.  Note that shared sockets (through dup/fork/exec) are
> counted as one socket. */
> -#define NUM_SOCKS   (32768 / sizeof (wsa_event))
> +#define NUM_SOCKS   ((unsigned int) 2048)
>  
>  #define LOCK_EVENTS  \
>if (wsock_mtx && \
> @@ -623,7 +623,14 @@ fhandler_socket::init_events ()
>NtClose (wsock_mtx);
>return false;
>  }
> -  wsock_events = search_wsa_event_slot (new_serial_number);
> +  if (!(wsock_events = search_wsa_event_slot (new_serial_number)));
> +{
> +  set_errno (ENOBUFS);
> +  NtClose (wsock_evt);
> +  NtClose (wsock_mtx);
> +  return false;
> +}
> +
>/* sock type not yet set here. */
>if (pc.dev == FH_UDP || pc.dev == FH_DGRAM)
>  wsock_events->events = FD_WRITE;
> -- 
> 2.8.3

Pushed.  I just changed

  #define NUM_SOCKS   ((unsigned int) 2048)

to

  #define NUM_SOCKS   2048U


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat


signature.asc
Description: PGP signature

[PATCH] Fix two bugs in the limit of large numbers of sockets:

[Erik M. Bray]

* Fix the maximum number of sockets allowed in the session to 2048,
  instead of making it relative to sizeof(wsa_event).

  The original choice of 2048 was in order to fit the wsa_events array
  in the .cygwin_dll_common shared section, but there is still enough
  room to grow there to have 2048 sockets on 64-bit as well.

* Return an error and set errno=ENOBUF if a socket can't be created
  due to this limit being reached.
---
 winsup/cygwin/fhandler_socket.cc | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc
index 7a6dbdc..b8eda57 100644
--- a/winsup/cygwin/fhandler_socket.cc
+++ b/winsup/cygwin/fhandler_socket.cc
@@ -496,7 +496,7 @@ fhandler_socket::af_local_set_secret (char *buf)
 /* Maximum number of concurrently opened sockets from all Cygwin processes
per session.  Note that shared sockets (through dup/fork/exec) are
counted as one socket. */
-#define NUM_SOCKS   (32768 / sizeof (wsa_event))
+#define NUM_SOCKS   ((unsigned int) 2048)
 
 #define LOCK_EVENTS\
   if (wsock_mtx && \
@@ -623,7 +623,14 @@ fhandler_socket::init_events ()
   NtClose (wsock_mtx);
   return false;
 }
-  wsock_events = search_wsa_event_slot (new_serial_number);
+  if (!(wsock_events = search_wsa_event_slot (new_serial_number)));
+{
+  set_errno (ENOBUFS);
+  NtClose (wsock_evt);
+  NtClose (wsock_mtx);
+  return false;
+}
+
   /* sock type not yet set here. */
   if (pc.dev == FH_UDP || pc.dev == FH_DGRAM)
 wsock_events->events = FD_WRITE;
-- 
2.8.3