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

Check that all network calls are done on the system thread #1040

Closed
monkbroc opened this issue Jun 20, 2016 · 2 comments

Comments

@monkbroc
Copy link
Member

commented Jun 20, 2016

#904 was due to a network function being called on the application thread. Let's audit the system firmware to check that all functions that perform network operations or modify system thread state are called on the system thread.

System.keepAlive seems suspect since it uses the ProtocolFacade.


Completeness:

  • Photon network calls reviewed
  • Electron network calls reviewed

@m-mcgowan m-mcgowan added this to the 0.6.x milestone Jun 23, 2016

@technobly technobly modified the milestones: 0.7.x, 0.6.x Aug 9, 2016

@m-mcgowan

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

Any code that is run on the system thread that is not threadsafe, should only ever run on the system thread. This typically means any calls to those functions are wrapped in the SYSTEM_THREAD macro.

We might also consider adding macros to the functions themselves that validate they are run on the correct thread in debug builds, so mistakes can be caught.

@m-mcgowan

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

Some possible approaches:

  • get a static analysis of the call stack from the system loop
  • get a static analysis of the call stack from all application API entrypoints (use the API testsuite, compiled against gcc.) Can validate all APIs are covered by using code coverage against the wiring module.
  • if a method on the application thread also exists in the callchain of the system thread then we have a potential issue of calling thread-unsafe code from multiple threads.

@technobly technobly modified the milestones: 0.7.x, 0.6.1 Nov 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.