Set() and remove() methods behaves unreliably on play 2.5 and older because of race conditions

[hanzki]

I noticed that the set() and remove() implementation of CacheAPI call async methods on spy.memcached.MemcachedClient which return futures but the futures are ignored. The default cache in play (prior to version 2.6) is synchronous and any replacement of the default implementation should conform to the same interface.

I ran into this problem when a value that was just saved with set() wasn't found with the following get() call. Because the CacheAPI interface uses Unit as the return type for set() and remove() the implementation of play2-memcached should await the futures returned by spy.memcached.MemcachedClient. The async methods could still be provided by explicitly calling the plugin with for example:

play.api.Play.current.plugin[MemcachedPlugin].get.asyncSet(...)

I can create a pull request for this but I'm not sure on which branch/commit should I base it on and to which branch should I target the PR such that this fix can be introduced to the versions meant for older play versions?

[mumoshu]

@hanzki Hey! Sorry for the loooong silence. This sounds like a problem. If you are still interested in doing so, submitting a PR against master branch is appreciated.

[mkurz]

@hanzki @mumoshu I think you want to submit a pull request against legacy branch, not against master. master "only" contains latest Play support (2.7), whereas legacy branch contains all other, older versions. EDIT: And this pull request needs to be done for Play 2.5 and older.

[mumoshu]

@mkurz Thanks! You are absolutely correct - seems like I'm losing my memory...

So probably we're talking about this: https://github.com/mumoshu/play2-memcached/blob/legacy/plugin/src/main/play_2.5/com/github/mumoshu/play2/memcached/MemcachedCacheApi.scala#L73

And this one: https://github.com/mumoshu/play2-memcached/blob/legacy/plugin/src/main/play_2.5/com/github/mumoshu/play2/memcached/MemcachedCacheApi.scala#L79