Closed cwakamo closed 6 years ago
• Explanation: Previous versions of PlaygroundLogger would never skip logging, even if a thread was already logging. This causes the side-effects of logging to be logged, which can cause infinite recursion in otherwise valid code. This change makes it so each thread tracks whether or not it is logging and returns nil if it already is logging, preventing this infinite recursion.
• Scope of Issue: Affects playgrounds which reference self
in CustomStringConvertible, CustomPlaygroundDisplayConvertible, or other protocols or methods consulted during logging.
• SR Issue: SR-8349
• Risk: The biggest risk here is that we may skip logging too many things, but the changes are very straightforward so I think this is a low risk.
• Testing: Added new tests to the PlaygroundLogger test suite which simulate this scenario.
• Reviewer: @jonathanpenn (in #29)
@tkremenek previously approved pulling this into swift-4.2-branch in email, so I'll be merging this shortly.
The logger entrypoints now consult a C-defined thread-local bool and exit immediately if the current thread is already logging. If the current thread is not logging, they set the bool to indicate that it is, and then unset it at the end of the function.
This allows types which reference self in their implementations of CustomPlaygroundQuickLookable or CustomStringConvertible (among other things) in such a way that self would be logged to not cause infinite recursion in the logger. It also means that logging won't log the side-effects of logging anymore.
This pulls the changes from #29 into swift-4.2-branch for a potential 4.2.x update, addressing rdar://problem/41460357 / SR-8349.