Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache stops being written to when java.io.InterruptedIOException: thread interrupted #1903

Closed
snodnipper opened this issue Oct 7, 2015 · 9 comments
Labels
bug Bug in existing code
Milestone

Comments

@snodnipper
Copy link
Contributor

When requesting tens of files in separate threads, cancelling some results in an java.io.InterruptedIOException: thread interrupted exception that causes hasJournalErrors = true;.

Consequently, within private synchronized Editor edit(String key, long expectedSequenceNumber) the cache is no longer updated because of hasJournalErrors.

Once in this state I do not believe that rebuildJournal(); is called until the cache is reinitialized. I looked in journalRebuildRequired() but that does not consider the hasJournalErrors flag.

Perhaps I should guard against threads being cancelled that are writing to the cache? RFC.

okhttp

@swankjesse
Copy link
Collaborator

@snodnipper are you calling Thread.interrupt() ? That's unexpected on threads performing HTTP requests with OkHttp. You should prefer Call.cancel() instead.

@swankjesse swankjesse added the bug Bug in existing code label Oct 31, 2015
@swankjesse swankjesse added this to the 2.6 milestone Oct 31, 2015
@swankjesse
Copy link
Collaborator

... or are you calling ExecutorService.shutdown() ?

@snodnipper
Copy link
Contributor Author

thanks Jesse for getting back. I am going to have to check early next week - the calling code is proprietary, where I simply provided an adapter using okhttp. I am pretty sure it is just an ExecutorService with boolean cancel(boolean mayInterruptIfRunning = true).

Essentially, the use case is:

  • user views a map which requests tens of map tiles (unique urls to cover visible map data)
  • user zooms in or pans
  • tiles scheduled for download but now outside of the visible area are cancelled.

I will try and provide some further feedback next week. Cheers!

@swankjesse
Copy link
Collaborator

Right now OkHttp doesn't do well with interrupted threads. If you can use Call.cancel instead, things will probably work better.

(And we should independently decide what to do about Thread.interrupt, if anything.)

@swankjesse swankjesse modified the milestones: 2.7, 2.6 Nov 1, 2015
@snodnipper
Copy link
Contributor Author

So the logic of the software is pretty simple:

  • ThreadPoolExecutor
  • Runnable objects are submitted to the executor and the Future is stored in a map (URL -> Future)
  • Futures are cancelled with cancel(true)

I resolved it quickly by using MapDB as the cache. I then discussed a few options with folk, e.g. letting writes continue with another thread / using write ahead logs etc. We didn't want to put too much effort in without at least checking in with you guys.

Given the above, I couldn't see a great place to use Call.cancel - the generic ExecutorService (within the external library) could be running anything to get data, such as reading straight from disk.

@schoenobates

@swankjesse
Copy link
Collaborator

@snodnipper got it. My guess is that Future.cancel() calls Thread.interrupt(), and OkHttp isn't prepared to deal with this.

@swankjesse swankjesse modified the milestones: 2.8, 2.7 Nov 22, 2015
@swankjesse swankjesse removed this from the 2.8 milestone Dec 15, 2015
@swankjesse swankjesse added this to the 3.1 milestone Jan 1, 2016
@snodnipper
Copy link
Contributor Author

added a crude project to demo the problem https://github.com/snodnipper/okhttp-issue1903-demo

@swankjesse swankjesse modified the milestones: Icebox, 3.3, 3.4 May 8, 2016
@swankjesse swankjesse modified the milestones: 3.6, 3.5 Oct 16, 2016
@swankjesse swankjesse modified the milestones: 3.7, 3.6 Dec 11, 2016
@cre8ivejp
Copy link

Any updates?
This is still happening on the version 3.8.0

@yschimke
Copy link
Collaborator

Dupe of #1902, closing as won't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in existing code
Projects
None yet
Development

No branches or pull requests

4 participants