Comments (12)
is this bug still here?
I'm getting lots of unexpected EOFs when sending big messages between two nodes, could it be relevant? EOFs start to happen when message size exceeds send_split_size
parameter
UPD: After several days of debugging, it turned out I didn't do socket.close().await
in OutboundUpgrade
.
from rust-yamux.
Thanks for the ping!
Regarding the linked socket.close().await
I am pretty sure it was due to related change in write_length_prefixed in rust-libp2p and this need for this was hinted in the rust-libp2p change as required migration ... I'll try to find it, sadly nothing of this was recorded in comments or commit msg. I would however not take rust-ipfs as an example @folex; I haven't had time to work on it for a long while now.
I'll update if/when I'll find more rationale for that close.
Oki, found it I think, originally write_one
was being used in that particular place, and it used to do the close
dance as well, so the linked commit was just about restoring the old behaviour without deprecated rust-libp2p code. The related libp2p commit is libp2p/rust-libp2p@c1ef4bf.
However to repeat, I cannot comment on if that close should be there, as my efforts have recently focused on trying to understand the latest windows test failures which started with the latest rust-libp2p upgrade. The bitswap protocol we have is nothing but a draft. I'll try to remember to add some disclaimers around it as well.
from rust-yamux.
Yes, indeed, there is a race condition here. Users of yamux
have to call poll_close
on a stream before dropping it to make sure all messages are sent before closing the stream with the remote.
Let me know in case that works for you.
from rust-yamux.
Yes, indeed, there is a race condition here. Users of
yamux
have to callpoll_close
on a stream before dropping it to make sure all messages are sent before closing the stream with the remote.Let me know in case that works for you.
For convenience of user, It seems that GC can close stream automatically when stream is dropped.
from rust-yamux.
By sending RST to the remote, you inform of the fact that the messages that the remote has sent on the substream might not have been received and processed. Sending a close message and immediately discarding the substream would be against that idea.
from rust-yamux.
By sending RST to the remote, you inform of the fact that the messages that the remote has sent on the substream might not have been received and processed. Sending a close message and immediately discarding the substream would be against that idea.
My mistake, when stream is dropped, Gc should reset stream after all queued message were sent.
from rust-yamux.
It's not about sending remaining messages, but about receiving the messages that the remote might be sending to you. In your specific situation, you know that the remote doesn't send anything, but that's not the case in the absolute.
from rust-yamux.
It's not about sending remaining messages, but about receiving the messages that the remote might be sending to you. In your specific situation, you know that the remote doesn't send anything, but that's not the case in the absolute.
You are right, we should send Ack alongside with RST frame if receive from the remote.
I think that Gc can do it also.
from rust-yamux.
is this bug still here?
I am not aware of it.
UPD: After several days of debugging, it turned out I didn't do socket.close().await in OutboundUpgrade.
Do I understand correctly that this resolves the above mentioned EOF on large messages? 🎉
from rust-yamux.
Do I understand correctly that this resolves the above mentioned EOF on large messages?
Yes! :)
While we're at it, could you kindly elaborate on why requirement to call socket.close()
appeared and why there wasn't such a requirement before?
I'm looking at e.g. rust-ipfs c7a79980bd and I see that they somehow understood that they should add socket.close()
.
I want to learn how they knew about this requirement, was it communicated or expressed somewhere in the docs?
Many thanks for your great work!
from rust-yamux.
While we're at it, could you kindly elaborate on why requirement to call
socket.close()
appeared and why there wasn't such a requirement before?I'm looking at e.g. rust-ipfs c7a79980bd and I see that they somehow understood that they should add
socket.close()
.I want to learn how they knew about this requirement, was it communicated or expressed somewhere in the docs?
Puuh, I am not sure to be honest. Maybe @koivunej knows more.
Many thanks for your great work!
❤️
from rust-yamux.
This bug can still be triggered if a user calls flush()
first and then drops the stream because at the moment, poll_flush
is a no-op. I have a fix for this in #156.
from rust-yamux.
Related Issues (20)
- Waiting on new channel and channel read in the same task HOT 2
- Detect if stream has ended on server side HOT 1
- add chunking in poll_write? HOT 1
- Use type state pattern HOT 4
- Simple client/server example blocks forever HOT 8
- Consider yielding early if we have filled buffers to some degree
- Track the accept backlog and add an upper limit HOT 3
- Re-write interface to buffer streams internally HOT 1
- Remove `futures::Stream` impl from `Stream`
- Remove `ControlledConnection` and everything around it
- Consider increasing `receive_window` default HOT 7
- Dropped `Stream`s are never removed until connection closes? HOT 1
- Refactor `stream::Shared` to use internal mutability
- Doc for v0.12.0 is outdated HOT 2
- refactor: replace `Chunks` with `BytesMut` HOT 2
- chore: remove `WindowUpdateMode::OnReceive` HOT 1
- What is the idiomatic way to create multiple streams? HOT 2
- `poll_new_outbound` without sending data results in no `inbound` stream HOT 2
- Is there any problem with my code and why it doesn't work HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rust-yamux.