Skip to content
This repository has been archived by the owner on Sep 28, 2021. It is now read-only.

Close down okhttp dispatcher executor #137

Merged
merged 1 commit into from
Jul 22, 2016
Merged

Conversation

rouzwawi
Copy link
Member

@rouzwawi rouzwawi commented Jul 22, 2016

This seems to be the way one should close the okhttp client according to: https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/OkHttpClient.java#L90-L100

I've tested this with a CLI that earlier would hang the JVM for about 30 seconds before actually exiting. Looking at the threads, I saw that one non-deamon thread called Dispatcher was not stopped.

@codecov-io
Copy link

codecov-io commented Jul 22, 2016

Current coverage is 66.07% (diff: 100%)

Merging #137 into master will decrease coverage by 1.60%

@@             master       #137   diff @@
==========================================
  Files           137        123    -14   
  Lines          2813       2550   -263   
  Methods           0          0          
  Messages          0          0          
  Branches        237        223    -14   
==========================================
- Hits           1904       1685   -219   
+ Misses          862        819    -43   
+ Partials         47         46     -1   

Powered by Codecov. Last update 999d16a...06d7e83

@spkrka
Copy link
Member

spkrka commented Jul 22, 2016

👍

@rouzwawi rouzwawi merged commit cad580c into spotify:master Jul 22, 2016
@mattnworb
Copy link
Member

this will be very helpful, thanks @spkrka

@spkrka
Copy link
Member

spkrka commented Jul 22, 2016

@mattnworb all credit goes to @rouzwawi , I just reviewed it. :)

@mattnworb
Copy link
Member

d'oh I meant to comment on #135 !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants