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

[Gen 3] Fixes mesh pub/sub socket consuming all packet buffers #1839

Merged
merged 8 commits into from Jul 29, 2019

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Jun 26, 2019

Problem

See #1828

Solution

This PR mainly simply starts mesh pub/sub thread that reads the data out of the socket even if only publish() is called.

Apart from that it:

  1. Refactors/cleans up MeshPublish class
  2. Provides a workaround for wiring Thread class where the thread being started was not given processing time due to os_thread_yield
  3. Enables mutlicast loop LwIP options which enables on-device testing of mesh pub/sub and adds the MESH_01_PublishWithoutSubscribeStillReadsDataOutOfPubSubSocket test.

Steps to Test

  • wiring/no_fixture: MESH_01_PublishWithoutSubscribeStillReadsDataOutOfPubSubSocket

Example App

N/A

References

  • Closes #1828
  • [CH35133]

Completeness

  • [bugfix] [Gen 3] Fixes mesh pub/sub socket consuming all packet buffers

@avtolstoy
Copy link
Member Author

@technobly If we are doing 1.3.0-rc.2, sounds like a good idea to put it in there instead of 1.3.1

return SYSTEM_ERROR_NO_MEMORY;
}

udp_ = std::move(udp);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clearing up the allocation leak.


// Use atomic exit_ as a simple synchronization primitive to ensure thread-safety
// instead of a mutex here.
if (exit_) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a race condition here between reading and setting the value - an atomic exchange will avoid this.

Suggested change
if (exit_) {
if (exit_.exchange(true) {

@avtolstoy avtolstoy force-pushed the fix/gen3-mesh-pubsub-packet-buffers branch from ad11f4e to 9808280 Compare July 24, 2019 09:43
@avtolstoy avtolstoy requested a review from m-mcgowan July 24, 2019 09:43
@avtolstoy avtolstoy added ready to merge PR has been reviewed and tested and removed needs review labels Jul 29, 2019
@avtolstoy avtolstoy force-pushed the fix/gen3-mesh-pubsub-packet-buffers branch from 9808280 to 24f4fa0 Compare July 29, 2019 13:31
@avtolstoy avtolstoy merged commit 3497493 into develop Jul 29, 2019
@avtolstoy avtolstoy deleted the fix/gen3-mesh-pubsub-packet-buffers branch July 29, 2019 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready to merge PR has been reviewed and tested
Projects
None yet
2 participants