[4.2] [PlaygroundLogger] Add checks in the logger entrypoints to prevent recursive logging. #30
Conversation
Added a thread-local `bool`, `PGLThreadIsLogging`, which is used in each of the logger entrypoints. This is defined in C as it's not possible (to my knowledge) to create a thread-local variable in Swift. Each of the logger entrypoints first checks this value; if it's `true`, then the function returns nil. If it's `false`, then the function sets it to `true`, defers setting it back to `false`, and then generates the log packet. This allows logging to complete successfully in cases when logging a value would cause that value to be logged (e.g. if the implementation of `CustomStringConvertible` or `CustomPlaygroundDisplayConvertible` caused `self` to be logged). This addresses <rdar://problem/41460357> / SR-8349. (cherry picked from commit c8af268)
Added tests for both the new and legacy entrypoints to ensure that recursive logging does not occur. In both cases, a struct whose implementation of `CustomStringConvertible` logs `self` is logged. Without the changes in the previous commit, this causes infinite recursion. With those changes, these tests pass as expected. This is for <rdar://problem/41460357> / SR-8349. (cherry picked from commit 58f43da)
…d set `PGLThreadIsLogging`. This is to insulate from any issues which might arise because Swift may or may not handle `__thread` variables correctly. (cherry picked from commit e2fcd53)
(cherry picked from commit 475d13a)
|
• 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. @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.