You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Because another thread could call logcat { "Well intended message" } between lines 1 and 2.
It's true that uninstall sets the logger to NoLog which says nothing is loggable. This would likely just drop any logcat{} logs that come in on the floor.. but... I was thinking that since the logcat{} calls delegate to the logger without locking on Logcat (which is an obvious choice and makes sense, you don't want to lock on that), but since we don't, I think it could be (looking at the implementation)
inline fun Any.logcat(
priority: LogPriority = DEBUG,
/**
* If provided, the log will use this tag instead of the simple class name of `this` at the call
* site.
*/
tag: String? = null,
message: () -> String
) {
LogcatLogger.logger.let { logger ->
// IF THE LOG SWAP TO NoLog HAPPENS AFTER THIS if isLoggable LINE FIRES
if (logger.isLoggable(priority)) {
val tagOrCaller = tag ?: outerClassSimpleNameInternalOnlyDoNotUseKThxBye()
// BUT BEFORE THIS logger.log LINE FIRES
logger.log(priority, tagOrCaller, message())
}
}
}
Then the NoLog will throw an exception
private object NoLog : LogcatLogger {
override fun isLoggable(priority: LogPriority) = false
override fun log(
priority: LogPriority,
tag: String,
message: String
) = error("Should never receive any log")
}
}
tl;dr;
I think we should offer a replaceLogger something like..
fun replaceLogger(newLogger: LogcatLogger) {
synchronized(this) {
uninstall()
install(newLogger)
}
}
And I think we should make NoLog gentler and something like
private object NoLog : LogcatLogger {
override fun isLoggable(priority: LogPriority) = false
override fun log(priority: LogPriority, tag: String, message: String) = Unit
}
}
The text was updated successfully, but these errors were encountered:
If you need to have two logger implementations and swap these out, you can just make a logger implementation that does that within itself.
The API was never meant to be used as "uninstall(), install()" as a way to swap impls, and I'd rather not relax NoLog just for a use case that the APIs aren't meant to support and that AFAIK can be implemented with a custom logger
I don't think it's safe to call
Because another thread could call
logcat { "Well intended message" }
between lines 1 and 2.It's true that uninstall sets the logger to NoLog which says nothing is loggable. This would likely just drop any logcat{} logs that come in on the floor.. but... I was thinking that since the logcat{} calls delegate to the logger without locking on Logcat (which is an obvious choice and makes sense, you don't want to lock on that), but since we don't, I think it could be (looking at the implementation)
Then the NoLog will throw an exception
tl;dr;
I think we should offer a replaceLogger something like..
And I think we should make NoLog gentler and something like
The text was updated successfully, but these errors were encountered: