Re: [PATCH v4] Make bindresvport() function to multithread-safe

[Rich Felker <dalias at aerifal dot cx>]

On Tue, Oct 16, 2012 at 05:33:51PM +0800, Peng Haitao wrote:
> 
> On 09/28/2012 10:27 PM, Carlos O'Donell wrote:
> >>  /*
> >>   * Bind a socket to a privileged IP port
> >>   */
> >>  int
> >>  bindresvport (int sd, struct sockaddr_in *sin)
> >>  {
> >> -  static short port;
> >> +  static __thread short port;
> > 
> > I'm curious how our relocation count changes with this?
> > 
> > Net neutral or did we gain extra relocations by changing this from
> > static to static __thread?
> > 
> > Could you have a look into that?
> > 
> 
> The static variable will cause bindresvport to unsafe.
> 
> Use thread A and thread B to explain:
> 
> thread A: when port is 1023, execute "if (port > endport)"
> thread B: execute "sin->sin_port = htons (port++);", port is 1024
> thread A: execute "res = __bind (sd, sin, sizeof (struct sockaddr_in));"
> 
> So, static should be replaced with static __thread.

Adding overhead to every thread in every program for the sake of a
nonstandard function that will almost never be used is not a good
design. Instead, it should simply be protected by a lock, or modified
atomically.

Rich