Mutex lock on flush#36
Merged
matsumo0922 merged 5 commits intomainfrom Jul 17, 2025
Merged
Conversation
This change allows PureeLogger to operate without a lifecycle, which is primarily relevant for iOS where a default lifecycle isn't always available or necessary.
On Android, PureeLogger continues to use ProcessLifecycleOwner by default.
Key changes:
- `DefaultLifecycleOwner` has been removed.
- `PureeLogger` now accepts a nullable `Lifecycle`. If null, it immediately calls `resume()`.
- The iOS `Puree.build()` method now passes `null` for the lifecycle.
- The Android `Puree` constructor now directly uses `ProcessLifecycleOwner.get().lifecycle`.
- `PureeBufferedOutput.resume()` is now a suspend function and uses a Mutex for `requestFlush` to prevent concurrent flush operations.
- Updated iOS sample app:
- Added a "Flush!" button to manually trigger flushing.
- Modified `OSLogBufferedOutput` to simulate a delay in `emit` for testing purposes.
- Adjusted `AddTimeFilter` to include fractional seconds in the timestamp.
This commit refactors how CoroutineScope is created and used within `PureeLogger` and `PureeBufferedOutput`.
- `PureeLogger`:
- Now directly accepts a `CoroutineContext` (renamed from `dispatcher`) in its constructor.
- The internal `scope` is initialized directly with this context.
- The `CoroutineExceptionHandler` and `SupervisorJob` are now configured at the platform-specific `Puree` builder level (`Puree.android.kt` and `Puree.ios.kt`) and passed into `PureeLogger`.
- `PureeBufferedOutput`:
- The `initialize` method now accepts a `CoroutineScope` directly instead of a `CoroutineContext`.
- This simplifies the initialization as the `CoroutineExceptionHandler` and `SupervisorJob` are now handled by the `CoroutineScope` provided by `PureeLogger`.
This change aims to centralize the CoroutineScope configuration at the builder level, making the core components more focused on their specific responsibilities and improving testability by allowing injection of a pre-configured scope.
matsumo0922
commented
Jul 17, 2025
| * @see resume | ||
| */ | ||
| internal suspend fun requestFlush() { | ||
| internal suspend fun requestFlush() = flushMutex.withLock { |
Member
Author
There was a problem hiding this comment.
Mutex を用いて二重に呼ばれないように調整します。
| coroutineScope.launch { | ||
| internal suspend fun resume() { | ||
| workerJob?.cancelAndJoin() | ||
| workerJob = coroutineScope.launch { |
Member
Author
There was a problem hiding this comment.
resume が二重に呼ばれないようにも調整します
| ) | ||
| if (lifecycle != null) { | ||
| lifecycle.addObserver( | ||
| object : DefaultLifecycleObserver { |
Member
Author
There was a problem hiding this comment.
iOS で onResume を返すだけの LifecycleOwner を作って使用していましたが、これが悪さしている可能性をなくすため、nullable にして ios では null を渡すようにしています。
| internal var dispatcher: CoroutineDispatcher = newSingleThreadContext("PureeLogger") | ||
| internal val dispatcher = newSingleThreadContext("PureeLogger") | ||
| .plus(SupervisorJob()) | ||
| .plus( |
Member
Author
There was a problem hiding this comment.
dispatcher を puree 全体で統一しています
This commit introduces a new output class, `PurgeableOSLogWarningBufferedOutput`, specifically for `PeriodicLog` types. Additionally, comments were updated in `OSLogBufferedOutput.swift` and minor formatting adjustments were made in `AddTimeFilter.swift`.
| internal fun resume() { | ||
| coroutineScope.launch { | ||
| internal suspend fun resume() { | ||
| workerJob?.cancelAndJoin() |
Member
There was a problem hiding this comment.
resume はどのスレッドから呼ばれる想定ですか？
もしメインスレッドで実行されていた場合、 cancelAndJoin によって workerJob が完了するまでメインスレッドをブロックしないか少し心配しています。
Member
Author
There was a problem hiding this comment.
https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/new-single-thread-context.html によると、newSingleThreadContext は占有のスレッドを新しく立ち上げて用いるようです。
そのためスレッドプールも IO や Main とは異なり、メインスレッドが割り当てられることはありません。
Member
There was a problem hiding this comment.
良さそう。調査ありがとうございました。
Member
Author
|
レビューありがとうございました 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related links
Why?
How?
flushが二重に走ることのないように、mutex でロックしますTesting 🔍