[skip ci] ween part 2: electric boogaloo

Reporter: time-killer-games  |  Status: open  |  Last Modified: June 29, 2020, 11:37:08 AM

TODO: how tf to make this async
@RobertBColton it appears the only way to make this async is to use the FILE_FLAG_OVERLAPPED flag via CreateFile, set ReadFile's lpNumberOfBytesRead parameter to NULL, and use the last parameter of ReadFile. But we aren't using CreateFile with this code, so not sure how I am supposed to do it? https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile

Also I removed one of our break's. I don't know if I was supposed to do that or not...


I'm not sure, let me do some more research.

I think it might be why Microsoft was using CreateFile in their example, it's possible to connect your pipe to a file.

Yes, you were correct to remove the break.


It seems as though it's just not possible with CreatePipe so you need to use CreateNamedPipe for the one we want to read. If it's simpler, you could try leaving the other pipes use CreatePipe since we are only interested in asynchronous behavior for the output pipe.

@time-killer-games In other words, the anonymous pipe you were using before? It was actually still a named pipe but just used the default name. Pipes are almost always backed by memory mapped files for interprocess communication. So even though you didn't realize it, because it was all abstracted away from you before, you were still in fact creating a file for the pipe!

A good example of this is how when you are using the Linux/Unix equivalents, you are working with file handles, even if you don't directly create a file. The pipe itself is a special file.


Sorry, I got sidetracked working on other things and then went to bed. I'm reading over your responses now.

@RobertBColton what do you suggest I do for this pr? I'd strongly prefer to just revert this back to fully working code, if we can't manage to find an approach you like better any time soon.

What are you talking about? This pull request was fine, all you needed to do was figure out the CreatedNamedPipe API and use the async version of ReadFile. I was waiting on you to do that.

I have no idea how to do anyone this, I can read the docs on those functions, but idk how that will tell me to do exactly what you are after. I'll try it though.

I can attempt it later if you want but I'm focusing on platforms atm.

You see the thing is, even if you put back your regular message pump you had from before, I think you are still going to hit the 4096 character limit, because you need to read while the process is running. So reverting your code is absolutely pointless here, it will just make it worse. You're better off to AT THE VERY LEAST keep the part we already fixed and just add back your message loop.

You have no other way to get around this, except the thread like I mentioned, which is even more painful than just figuring out CreateNamedPipe.


I've exceeded that many characters significantly though. I'll take your word for it. You understand that stuff a lot better than my crappy guess work and copy pasta.

Codecov Report

Merging #2031 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2031      +/-   ##
+ Coverage   30.94%   30.95%   +0.01%     
  Files         197      197              
  Lines       19100    19115      +15     
+ Hits         5910     5918       +8     
- Misses      13190    13197       +7     
Impacted Files Coverage Δ
...MAsystem/SHELL/Widget_Systems/None/default_log.cpp 41.66% <0.00%> (-4.49%) ⬇️
ENIGMAsystem/SHELL/Platforms/SDL/Window.cpp 58.82% <0.00%> (-1.42%) ⬇️
ENIGMAsystem/SHELL/Platforms/xlib/XLIBwindow.cpp 51.66% <0.00%> (-0.69%) ⬇️
...NIGMAsystem/SHELL/Universal_System/terminal_io.cpp 5.55% <0.00%> (-0.33%) ⬇️
...GMAsystem/SHELL/Platforms/xlib/XDisplayGetters.cpp 92.59% <0.00%> (ø)
ENIGMAsystem/SHELL/Platforms/General/PFwindow.cpp 41.30% <0.00%> (+0.87%) ⬆️
ENIGMAsystem/SHELL/Platforms/xlib/XLIBmain.cpp 37.03% <0.00%> (+5.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dab5494...59a765d. Read the comment docs.

Please sign in to post comments, or you can view this issue on GitHub.