network_ppp: Stop polling if stream or PPP PCB are removed. #17904
+4
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Code size report: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17904 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 171 171
Lines 22297 22297
=======================================
Hits 21938 21938
Misses 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dpgeorge
reviewed
Aug 15, 2025
If while a polling operation is active the stream is removed we should stop polling data from that stream. This was already the intended behaviour, but implemented incorrectly. Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
When disconnecting from PPP the modem sends a confirmation. This message is received, like all messages, through the poll() method. lwIP may then immediately call our status callback with code PPPERR_USER to indicate the connection was closed. Our callback then immediately proceeds to free the PCB. Thus, during each new iteration of the loop in poll() we must check if we haven't disconnected in the meantime to prevent calling the pppos_input_tcpip with a PCB that is now NULL. Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
939e7a0 to
b764d01
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Two small fixes that improve when the
poll()method stops looping trying to read more data from the stream:None.streamto None, but I didn't correctly port it over to the extmod version. And then later I brought the bug from the extmod version back to the ESP32 version. 😞poll()method. lwIP may then immediately call our status callback with codePPPERR_USERto indicate the connection was closed. Our callback then immediately proceeds to free the PCB. Thus, during each new iteration of the loop inpoll()we must check if we haven't disconnected in the meantime to prevent calling thepppos_input_tcpipwith a PCB that is now NULL.Testing
Tested on the ESP32 by disconnecting and observing we no longer crash due to
self->pcbbeingNULL.Updated the extmod version to keep the two versions in sync. Haven't tested it on a port using the extmod version, but it looks like a correct condition to check there as well.