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

Handle partial transmits in WSASend (Compatibility with WINE). #28524

Closed
Schramp opened this issue Jan 11, 2022 · 4 comments
Closed

Handle partial transmits in WSASend (Compatibility with WINE). #28524

Schramp opened this issue Jan 11, 2022 · 4 comments
Assignees
Labels
disposition/help wanted Maintainers do not have enough resources to allocate to this at the moment. Help is appreciated! kind/bug lang/core priority/P2

Comments

@Schramp
Copy link

Schramp commented Jan 11, 2022

What version of gRPC and what language are you using?

Python 3.9, Latest (For testing)

commit 10b2b50 (schramp/master, origin/master, origin/HEAD, master)
Date: Fri Jan 7 13:04:21 2022 -0800

What operating system (Linux, Windows,...) and version?

Windows . (WINE emulation on Linux)

What runtime / compiler are you using (e.g. python version or version of gcc)

Python 3.9

C:\Users\Ruud\grpc>C:\Python39-32\python.exe -m pip list
Package Version


coverage 6.2
Cython 0.29.26
pip 21.2.4
protobuf 3.19.1
setuptools 58.1.0
six 1.16.0
wheel 0.37.1

Compiled with Visual Studio 2019

What did you do?

Send multiple messages (>1Mb) at a fast rate leading to TCP-Congestion

What did you expect to see?

Messages being transfered at high speed

What did you see instead?

After the TCP-Congestion window closes the WSASend does a short write, and reports no error (so no WSAEWOUDLBLOCK), after that the connection stalls. Measuring the tcpconnection using wiresharl shows only the data of the short write is transmitted, but the remainder of the buffer is not present. At that moment the connection stalls.

Anything else we should know about your project / environment?

This is a duplicate of #28472 but I can't change the labels.
I tried to get some feedback on https://groups.google.com/g/grpc-io/c/-RzRZ867jy0 but no responses.

I prepared a PR: #28506

log_339ad24501c1_head.gz

Please note that in the logging it doesn't break as its the patched version. grep for the word "REST" to go to the failing location. You can cross ref the patch for understanding of this log.

@drfloob
Copy link
Member

drfloob commented Jan 14, 2022

Thanks for the investigation, and apologies for the lack of responses. You are working in an area that requires a cross-section of expertise that nobody on the team has at the moment ... we don't build or test gRPC in WINE. I see that you called out a few issues in your PR, we'd be happy to review it when you think it's ready if there are no ill effects on Windows. Otherwise, I'd recommend using Linux-native gRPC on Linux if possible. I'm labelling this as "help wanted" in case the community wants to provide stronger support for this scenario.

@drfloob drfloob added the disposition/help wanted Maintainers do not have enough resources to allocate to this at the moment. Help is appreciated! label Jan 14, 2022
@Schramp
Copy link
Author

Schramp commented Jan 15, 2022

Thanks @drfloob, I am happy to improve the PR. But I don't want to invest time, if there is no willingness from the community to read and comment on the direction of the PR.

I would first discuss with "someone" on what I think is a bug that is made visible by using WINE, may actually be a bug that also exists in Windows but is rarer there.

If someone says "please continue", I will.

I suspect actual multiple bugs, this PR addresses only the first:

  • WSASend may do a short write, which isn't tested nor handled
  • WSASend may work async and use information on the stack, which is essentialy using "out of scope" memory that could be corrupted.

The usage of async calls with buffers on the stack feels like risky to me, but I would like some expert eg. @ctiller to comment on my code-comments before I continue. Making a PR that puts the info on a heap and make a LPWSAOVERLAPPED_COMPLETION_ROUTINE would be a task I could prepare, but only if we could agree that someone would be willing to shepherd the process from the core team.

Besides you remark on using windows, you are right! But the context of the system I work at doesn't have a windows/docker/kubernetes components (yet) and using WINE allows me to integrate native windows DLL's in the existing framework on which I have little controll.

@Schramp
Copy link
Author

Schramp commented May 17, 2022

Just to let you know I still check once in a while

@Schramp
Copy link
Author

Schramp commented Nov 21, 2022

Closed in #28432

@Schramp Schramp closed this as completed Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition/help wanted Maintainers do not have enough resources to allocate to this at the moment. Help is appreciated! kind/bug lang/core priority/P2
Projects
None yet
Development

No branches or pull requests

3 participants