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

[Photon/P1/Electron] Recursive logging freezes the application thread while the LogHandler is trying to acquire a lock on the resource. #1517

Merged
merged 4 commits into from Apr 5, 2018

Conversation

@technobly
Copy link
Member

commented Apr 5, 2018

Problem

Recursive logging freezes the application thread while the LogHandler is trying to acquire a lock on the resource (#1497).

Solution

  1. Use a RecursiveMutex in spark_wiring_logging.h
  2. Prevent re-entry into subclassed logHandler functions logMessage and logWrite from the LogManager instance

Steps to Test

Run TEST=wiring/no_fixture

References


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • [N/A] Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@technobly technobly added the bug label Apr 5, 2018

@technobly technobly added this to the 0.8.0-rc.3 milestone Apr 5, 2018

@technobly technobly requested a review from sergeuz Apr 5, 2018

@sergeuz
sergeuz approved these changes Apr 5, 2018
@@ -577,14 +577,16 @@ class LogManager {

Vector<LogHandler*> activeHandlers_;

volatile bool output_active_;

This comment has been minimized.

Copy link
@sergeuz

sergeuz Apr 5, 2018

Member

I know we tend to mix different styles in the code, but let's at least make sure that the code within a single file follows the same naming convention :) Also, volatile isn't really necessary here, since the access to this member variable is already guarded by a mutex.

This comment has been minimized.

Copy link
@technobly

technobly Apr 5, 2018

Author Member

I started with the word active_ and then thought it would be better to say what exactly is active. It wasn't my intention to mix snake_case with camelCase. I'll fix it :)

@technobly technobly changed the title Recursive logging freezes the application thread while the LogHandler is trying to acquire a lock on the resource. [Photon/P1/Electron] Recursive logging freezes the application thread while the LogHandler is trying to acquire a lock on the resource. Apr 5, 2018

@technobly technobly merged commit ee54fe3 into develop Apr 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@technobly technobly deleted the fix/recursive-logging branch Apr 13, 2018

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