Skip to content

Commit

Permalink
Don't use daemon threads in MockWebServer
Browse files Browse the repository at this point in the history
This was a regression introduced with the TaskRunner changes.
I couldn't find other places where daemon threads were likely
to cause potential problems.

#5512
  • Loading branch information
squarejesse committed Dec 7, 2019
1 parent 7e48705 commit d26bfca
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ import javax.net.ssl.X509TrustManager
*/
class MockWebServer : ExternalResource(), Closeable {
private val taskRunnerBackend = TaskRunner.RealBackend(
threadFactory("MockWebServer TaskRunner", true))
threadFactory("MockWebServer TaskRunner", daemon = false))
private val taskRunner = TaskRunner(taskRunnerBackend)
private val requestQueue = LinkedBlockingQueue<RecordedRequest>()
private val openClientSockets =
Expand Down Expand Up @@ -391,10 +391,10 @@ class MockWebServer : ExternalResource(), Closeable {

taskRunner.newQueue().execute("MockWebServer $portField", cancelable = false) {
try {
logger.info("${this@MockWebServer} starting to accept connections")
logger.info("$this starting to accept connections")
acceptConnections()
} catch (e: Throwable) {
logger.log(Level.WARNING, "${this@MockWebServer} failed unexpectedly", e)
logger.log(Level.WARNING, "$this failed unexpectedly", e)
}

// Release all sockets and all threads, even if any close fails.
Expand Down Expand Up @@ -468,10 +468,9 @@ class MockWebServer : ExternalResource(), Closeable {
try {
SocketHandler(raw).handle()
} catch (e: IOException) {
logger.info("${this@MockWebServer} connection from ${raw.inetAddress} failed: $e")
logger.info("$this connection from ${raw.inetAddress} failed: $e")
} catch (e: Exception) {
logger.log(Level.SEVERE,
"${this@MockWebServer} connection from ${raw.inetAddress} crashed", e)
logger.log(Level.SEVERE, "$this connection from ${raw.inetAddress} crashed", e)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package okhttp3.internal.concurrent
import okhttp3.internal.addIfAbsent
import okhttp3.internal.assertThreadDoesntHoldLock
import okhttp3.internal.assertThreadHoldsLock
import okhttp3.internal.concurrent.TaskRunner.Companion.INSTANCE
import okhttp3.internal.notify
import okhttp3.internal.threadFactory
import java.util.concurrent.SynchronousQueue
Expand All @@ -28,9 +29,8 @@ import java.util.concurrent.TimeUnit
/**
* A set of worker threads that are shared among a set of task queues.
*
* The task runner is responsible for managing non-daemon threads. It keeps the process alive while
* user-visible (ie. non-daemon) tasks are scheduled, and allows the process to exit when only
* housekeeping (ie. daemon) tasks are scheduled.
* Use [INSTANCE] for a task runner that uses daemon threads. There is not currently a shared
* instance for non-daemon threads.
*
* The task runner is also responsible for releasing held threads when the library is unloaded.
* This is for the benefit of container environments that implement code unloading.
Expand Down Expand Up @@ -289,6 +289,6 @@ class TaskRunner(

companion object {
@JvmField
val INSTANCE = TaskRunner(RealBackend(threadFactory("OkHttp TaskRunner", true)))
val INSTANCE = TaskRunner(RealBackend(threadFactory("OkHttp TaskRunner", daemon = true)))
}
}

0 comments on commit d26bfca

Please sign in to comment.