bugFreeciv - Bugs: bug #21583, Client can connect to manually...

 
 
Show feedback again

bug #21583: Client can connect to manually started server rather than its own spawned one

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sun 02 Feb 2014 02:56:08 PM UTC  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: S2_4 r24327Operating System: Microsoft Windows
Planned Release: 2.5.0-beta1, 2.6.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 01 Jun 2014 10:32:16 AM UTC, SVN revision 24996:

Restored SO_REUSEADDR for lan scan sockets.

See bug #21583

(Browse SVN revision 24996)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun 01 Jun 2014 10:32:10 AM UTC, SVN revision 24995:

Restored SO_REUSEADDR for lan scan sockets.

See bug #21583

(Browse SVN revision 24995)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 30 May 2014 12:13:40 AM UTC, comment #14:

Attached patch restores SO_REUSEADDR for lan scan sockets.

(file #20860)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 29 May 2014 10:52:41 PM UTC, comment #13:

Apparently some sockets should have SO_REUSEADDR while others not. Currently lan announcements are broken as multiple games cannot open announcement socket.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 29 May 2014 10:03:23 PM UTC, SVN revision 24974:

Do not set SO_REUSEADDR socket option.

Reported by Jacob Nevins

See bug #21583

(Browse SVN revision 24974)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 29 May 2014 10:03:17 PM UTC, SVN revision 24973:

Do not set SO_REUSEADDR socket option.

Reported by Jacob Nevins

See bug #21583

(Browse SVN revision 24973)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 22 Apr 2014 07:27:07 PM UTC, comment #10:

Let's not risk this in a release branch, but let's test it in 2.5 beta series.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon 17 Mar 2014 01:29:09 AM UTC, comment #9:

- Remove SO_REUSEADDR from client side sockets too.

(file #20364)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 15 Mar 2014 01:03:57 AM UTC, comment #8:

> A conversation with a friend makes me wonder whether our use of
> SO_REUSEADDR might be implicated.


From the article it sounds so. In fact, I can't understand why has SO_REUSEADDR ever been introduced there - doesn't sound something we would ever need. Attached patch simply removes it. At least in S2_4 I would be inclined to leave patch #4471 workaround to stand in addition to this, even if this seems to fix the actual problem. From the article it seems that the fix might not work on some version of Windows. Patch #4471 in turn does not fix reclaiming of a socket in cases other than one manually started server in default port + one client spawned server, so having both gives maximum stability we can reach.

(file #20336)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon 03 Feb 2014 04:01:49 PM UTC, comment #7:

> Obvious low-risk workaround for 2.4.1:


s/2.4.1/2.4.2.

This is now patch #4471.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon 03 Feb 2014 06:45:30 AM UTC, comment #6:

> find_next_free_port() erronously returned 5557 in 2.4.1 despite
> finding 5556 being available


Obvious low-risk workaround for 2.4.1: Start looking for the port for client-spawn server from 5557, so in most cases it will get 5557 like it used to.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon 03 Feb 2014 06:41:47 AM UTC, comment #5:

> This is one reason why changing anything in this area scares
> me, especially just before a release.


Yes. The question is whether we consider this current problem a reggression from 2.4.1?
The bug itself certainly isn't, but the fact that find_next_free_port() erronously returned 5557 in 2.4.1 despite finding 5556 being available made this much less likely to happen.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon 03 Feb 2014 01:08:37 AM UTC, comment #4:

> can't reproduce on Linux

No surprise -- I think this stuff tends to be horribly OS-dependent.

This is one reason why changing anything in this area scares me, especially just before a release.

Another similar source of fun is whether listening on a given port on IPv6 also exclusively binds on IPv4. We had lots of fun with this around bug #15559. (In that case at least it was also configurable, with different Linux distros choosing different defaults at different times; and *BSDs were different again.)

A conversation with a friend makes me wonder whether our use of SO_REUSEADDR might be implicated. This article (which I've only skimmed so far) looks interesting. (But this could be a complete red herring.)

>> Quitting the client then killed the manually started server
>> (unsurprisingly).
> How that? The server should wait for (re)connects, and does
> so here.

I think client_kill_server() is called, which sends "/quit" to what it thinks is its spawned server. Since the client always ends up with "hack" access to a server on the same machine (indeed, this can't be disabled: bug #20556) this succeeds. (Or at least I'm guessing that's what happened, hence my lack of surprise.)

Jacob Nevins <jtn>
Project Administrator
Sun 02 Feb 2014 04:25:36 PM UTC, comment #3:

I think this has to do with #21547.

> I guess the problem is that Freeciv assumes it's not possible for two processes to listen on 0.0.0.0:5556 and 127.0.0.1:5556 simultaneously, or something.
> (Can I just note that I hate interface binding semantics.)


didn't know that and can't reproduce on Linux. 2 servers, at 127... and 192..., is possible, but 1 server on 0... blocks all.

> Quitting the client then killed the manually started server (unsurprisingly).


How that? The server should wait for (re)connects, and does so here.

Christian Knoke <chrisk>
Project Member
Sun 02 Feb 2014 03:10:57 PM UTC, comment #2:

> How fast one after another the programs were started?

The manually started server had been running for some time (maybe minutes?) so I doubt it's a straightforward port claiming race.

Jacob Nevins <jtn>
Project Administrator
Sun 02 Feb 2014 03:03:57 PM UTC, comment #1:

How fast one after another the programs were started? I just wonder if this could be race-situation that the manually started server had not yet reserved the port by the time client checks if it's available.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun 02 Feb 2014 02:56:08 PM UTC, original submission:

Observed while verifying fix for bug #21547 with cproc's S2_4 r24327 Windows test build:

I had a freeciv-server.exe running (port 5556, started from start menu), started a freeciv-gtk2.exe, and accidentally chose "new game" rather than "connect to network game".

To my surprise, it connected to my manually-started server rather than one of its own.

Didn't seem 100% reproducible but I got it to happen twice in a few tries. Details from the second time:

There were two freeciv-server.exe processes, unsurprisingly.

netstat -a -b -o showed:

PID 3504 is presumably my standalone server and 3140 the spawned one.

I guess the problem is that Freeciv assumes it's not possible for two processes to listen on 0.0.0.0:5556 and 127.0.0.1:5556 simultaneously, or something.
(Can I just note that I hate interface binding semantics.)

Quitting the client then killed the manually started server (unsurprisingly).

After quitting the manual server, the spawned server hung around (unsurprisingly), but it didn't obviously cause trouble (a new client-spawned server used 5557).
(This might have been behind my lack of reproducibility, I didn't check again after I realised this was happening.)

(Windows 7 32-bit. Machine not connected to any "real" network, if it makes a difference.)

Dunno if this is 2.4.2 blocker -- case of manual server and local game simultaneously is quite niche, and messing around with this might break something else.

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #20860:  LanScanSoReuseAddr.patch added by cazfi (1kB - text/x-diff)
file #20364:  SoReuseAddrRm-2.patch added by cazfi (2kB - text/x-diff)
file #20336:  SoReuseAddrRm.patch added by cazfi (1019B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by chrisk (Posted a comment)
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 16 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 01 Jun 2014 10:32:25 AM UTCcazfiStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Fri 30 May 2014 12:13:40 AM UTCcazfiAttached File-=>Added LanScanSoReuseAddr.patch, #20860
      StatusIn Progress=>Ready For Test
    Thu 29 May 2014 10:52:41 PM UTCcazfiStatusFixed=>In Progress
      Open/ClosedClosed=>Open
    Thu 29 May 2014 10:03:37 PM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Tue 22 Apr 2014 07:27:07 PM UTCcazfiPlanned Release2.4.3, 2.5.0, 2.6.0=>2.5.0-beta1, 2.6.0
    Mon 17 Mar 2014 01:29:09 AM UTCcazfiAttached File-=>Added SoReuseAddrRm-2.patch, #20364
    Sat 15 Mar 2014 01:03:57 AM UTCcazfiAttached File-=>Added SoReuseAddrRm.patch, #20336
      StatusIn Progress=>Ready For Test
    Mon 03 Feb 2014 04:01:49 PM UTCcazfiCategoryNone=>general
      StatusNone=>In Progress
      Planned Release=>2.4.3, 2.5.0, 2.6.0
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup