Godot: Can't start project from editor

[lekoder]

Godot version:
3.1 ef7b38486

OS/device including version:
Windows 10

Issue description:
After clicking "play" project does not start.

Console shows:

Running: D:\Projects\godot\godot-src\bin\godot.windows.opt.tools.64.exe --path D:/Projects/godot-issues/interia-2d-problem --remote-debug 127.0.0.1:6007 --allow_focus_steal_pid 9932 --position 448,240
ERROR: NetSocketPosix::poll: Condition ' ret < 0 ' is true. returned: FAILED
   At: drivers\unix\net_socket_posix.cpp:356
ERROR: ScriptDebuggerRemote::connect_to_host: Remote Debugger: Unable to connect. Status: 3
   At: core\script_debugger_remote.cpp:92
ERROR: NetSocketPosix::_get_socket_error: Socket error: 10054
   At: drivers\unix\net_socket_posix.cpp:191

Steps to reproduce:
Start any project (including new empty project).

Minimal reproduction project:
n/a

[OliPerraul]

I am getting the same issue after pulling from master. (Same spec)

[Xrayez]

Same, I got slightly different error though:

ERROR: poll: Condition ' ret < 0 ' is true. returned: FAILED
   At: drivers/unix/net_socket_posix.cpp:356
ERROR: connect_to_host: Remote Debugger: Unable to connect. Status: 3
   At: core/script_debugger_remote.cpp:92

Caused by this PR: https://github.com/godotengine/godot/pull/22316 by @Faless

[capnm]

No problems with Linux.

[volzhs]

I confirm that no problem with linux (kubuntu 18.04)
several error message on console though.

ERROR: getornull: Condition ' !id_map.has(p_rid.get_data()) ' is true. returned: __null
   At: ./core/rid.h:165.

[lekoder]

I'm confirming what @Xrayez asserted: a384cffc4 - which is merge of #22316 by @Faless causes the issue.

[Faless]

Did I ever say I hate Windows? I guess I did. I'll investigate

[lekoder]

@Faless your problem is here. I don't claim to know what's going on there, but I bisected your commits with comments until I got the engine to work. It seems that err comes out as something different than OK or ERR_BUSY on Windows. HTH.

This works for me.

[Faless]

You mean disabling poll write? That might be an option, I'm no sure it's really needed.
In the error message from OP, it seems that windows is returning WSAECONNRESET (10054) when trying to connect.

[lekoder]

Well, I'm the OP :)
I did not look in-depth what is going on, but removing the checks fixes the issue for me. Mind you - I'm just bisecting your code without analyzing much. If I knew why that is happening, I would submit a PR.

[lekoder]

if (err != OK && err != ERR_BUSY && err != WSAECONNRESET ) {

also works, but doesn't that defeat the purpose of this check?

[Faless]

Well, I didn't notice :crying_cat_face: .
Thanks for reporting, and bisecting. I'll do some more tests trying to see what's going on.
Especially, I'm trying to understand why poll write is failing, and if that's something I need to handle at net_socket level instead.

[lekoder]

I'm trying to understand why poll write is failing

I'm betting my money on race condition. Like game doesn't start yet before debugger tries to connect to it. This should be probably elsewhere, your check here just exposed the problem.

[Faless]

What's even more awful is that if I run the windows version with wine on Linux it works.
So it really seems something with the underlying winsock behaviour.

[lekoder]

Does removing that check defeat purpose of #22316?

[Faless]

I'm honestly not sure if the poll write check is really needed (it can detect if the write part was closed, which should never happen in CONNECTED state, but I'm not 100% sure an underlying OS network error cannot cause that).
But in any case I'm not satisfied with the ostrich algo,
I'd prefer to understand what's actually going on, and if the investigation results in knowing we can't poll write, then remove it.

[lekoder]

Understood and agreed.
I would, however, vote for removing the check right now for sake of having functional Windows editor build in master - and opening separate branch/issue for the investigation.

[Faless]

Well, to be honest, I think we can wait few hours.
The issue is here, if one wants to use master can revert that commit in their branch.
master can be broken for short periods of time.
Of course, if the investigation take 2 days, then a quick fix can be an option.
But in this case I think I can wrap my head around this in few hours.

[lekoder]

Agreed. Best of luck!

[capnm]

master can be broken for short periods of time

Completely broken is bad later for bisecting other issues. Luckily I don't care about windows ;-)

[lekoder]

Completely broken is bad later for bisecting other issues

I can agree on the weekend, I can work from my own branch. Honestly, if anyone works of the master they should be able to apply patch from this conversation, or just pulling from my branch if there is a problem.

Immediate viability of master is a project-to-project decision and not ours to make. I like masters of my projects to be usable by default and all works-in-progress to be delegated to branches, but that's a maintainer call and we should trust their decision.

[capnm]

they should be able to apply patch from this conversation

I meant – such unusable state then makes git bisect almost impossible.

[Faless]

Seems like WSAPoll is broken by design.
MS wrote (long time ago) we should all move to that: https://blogs.msdn.microsoft.com/wndp/2006/10/26/wsapoll-a-new-winsock-api-to-simplify-porting-poll-applications-to-winsock/

But they then apparently decided to keep the broken implementation they have: https://daniel.haxx.se/blog/2012/10/10/wsapoll-is-broken/

I'll likely have to change the code to use select instead.

[Faless]

There, PR is out. Testers welcome.

[lekoder]

@Faless confirming it works as intended.