Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible for client to be in a nonexistent company on a server; but then the client crashes. #6598

Closed
DorpsGek opened this issue Aug 4, 2017 · 6 comments · Fixed by #9163
Labels
bug Something isn't working flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Aug 4, 2017

james1101 opened the ticket and wrote:

To Reproduce this Crash:
Start OTTD server.
Remember it's ip address.
Start OTTD client.
Client: Connect to server.
Client: Execute 'join 255'.
-> Client joins spectators.
Client: Execute 'join N', where N is greater than 15 and less than 255.
-> Client throws: 'Error: Company does not exist.'
Client: Execute 'connect # N', where N is greater than 15 and less than 255.
-> Client: Connect should be successful.
Client: Open company list.
-> Client crashes. Crash files are attached.

Client version 1.7.1 on Windows 7 Ultimate; Server version 1.7.1 on Windows 7 Ultimate

Suggested Fix: Make the company id part of the connect command's argument follow the same rules as the join command's company id argument, ie must be between 0 and max_companies, or 255 (spectator).

Attachments

Reported version: 1.7.1
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/6598
@DorpsGek
Copy link
Member Author

DorpsGek commented Aug 15, 2017

Wolf01 wrote:

I can't reproduce it on Win 10.
Tried with local server both with IPv4 and IPv6.
Tried logging in as a company 1 and spectator.
Tried connecting from titlescreen.

The command already checks for join_as > MAX_COMPANIES:

 if (join_as != COMPANY_SPECTATOR) {
 	if (join_as > MAX_COMPANIES) return false;
 	join_as--;
 }

This comment was imported from FlySpray: https://bugs.openttd.org/task/6598#comment14523

@DorpsGek
Copy link
Member Author

dp wrote:

Ok, I didn't get any crashes, but I think it's close enough. Thing is connect cmd sometimes leaves the game in a weird half-disconnected state. It's not very noticable at first but it's clearly not a valid state. E.g. you can build rails but can't leave company or use join command. Or you can bring up mp lobby while still being in game.
There are several ways to get in that state:

  1. connect ::1# 254 (or anything 16-254)
  2. connect ::1# 1 and cancelling password prompt (company 1 has to be passworded)
  3. connect ::1:0

I think I got all such cases in my patch but can't be 100% sure.
Also rearranged console output a bit so it doesn't say "connecting ..." before failing on arg parsing.
And added check for negative company # to prevent possible bugs with it.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6598#comment14526

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) labels Apr 7, 2018
@TrueBrain
Copy link
Member

Patch contains many things I am unsure are part of the solution. But it is a good start for solving this issue :)

@TrueBrain TrueBrain added good first issue Good for newcomers patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay bug Something isn't working and removed bug from FlySpray labels Apr 12, 2018
@frosch123 frosch123 removed the Core label Apr 14, 2018
@andythenorth andythenorth added the stale Stale issues label Jan 5, 2019
@andythenorth
Copy link
Contributor

Thanks for this. Valid issue, but there's been no activity on this for some time, and as it stands, it doesn't look likely that it will go any further. I'm closing it as we try to keep the issue count low for OpenTTD, it helps us focus on things that are important and fun. Feel free to discuss in irc or request re-opening if you disagree. Thanks for contributing!

@LordAro LordAro removed patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay stale Stale issues labels Jan 12, 2019
@LordAro LordAro reopened this Jan 12, 2019
@nikolas
Copy link
Member

nikolas commented Aug 6, 2019

Here's a crash.log triggered by a related scenario:

  • Enable an AI player to start in the game immediately. I used AdmiralAI.
  • Host a game, and put a password on it
  • From a second client, join the game, and join that company. (connect ::1#1 from the console.)
  • In the console of the second client, do: connect ::1#1 (effectively trying to join the same company). Don't type in the server password yet.
  • In this state, where the server password is being prompted, you can crash the second client in any number of ways. The first ones I've found: Open up the Company window, and try and change the company name, or president's name. Cancelling the password dialog box causes a crash as well.

Additionally, writing the crash savegame fails here (resolved by #7684):

Writing crash savegame...
Error: Assertion failed at line 286 of /home/nik/src/OpenTTD/src/ai/ai_core.cpp: c != nullptr && c->ai_instance != nullptr
Aborted

nikolas added a commit to nikolas/OpenTTD that referenced this issue Aug 6, 2019
This fixes an assertion failure created by the scenario
outlined here:
OpenTTD#6598 (comment)

When a client gets disconnected from a network game that has an AI, and
then crashes, it tries to write a crash.sav. Attempting to save the AI
instance here, which doesn't exist on the client, causes the assertion
failure.

This change allows openttd to crash properly in this awkward scenario :P

crash.sav and crash.png are now written:

    Writing crash log to disk...
    Crash log written to /home/nik/.openttd/crash.log. Please add this file
    to any bug reports.

    Writing crash savegame...
    Crash savegame written to /home/nik/.openttd/crash.sav. Please add this
    file and the last (auto)save to any bug reports.

    Writing crash screenshot...
    Crash screenshot written to /home/nik/.openttd/crash.png. Please add
    this file to any bug reports.

    Aborted
@Foxar
Copy link

Foxar commented Nov 29, 2020

Since this is marked as good first issue and I'm a newbie, I'd love to try and help out with this, could anyone point me in the right direction? :)

@TrueBrain TrueBrain removed pinned good first issue Good for newcomers labels Jan 3, 2021
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
rubidium42 added a commit that referenced this issue May 1, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
rubidium42 added a commit that referenced this issue May 1, 2021
…m within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 1, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 1, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 1, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 1, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 1, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 2, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 2, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 2, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
LordAro pushed a commit to LordAro/OpenTTD that referenced this issue May 2, 2021
…oin from within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
LordAro pushed a commit that referenced this issue May 3, 2021
NetworkClientConnectGame already does a NetworkDisconnect, so no reason to do it here
LordAro pushed a commit that referenced this issue May 3, 2021
…m within a network game

One could join a network game from within an already running network game. This would call a NetworkDisconnect, but keeps the UI alive. If, during that process the join is aborted, e.g. by cancelling on a password dialog, you would still be in your network game but also get shown the server list.
Solve all the underlying problems by falling back to the main UI when (re)connecting to a(nother) server.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants