[RFC][Icedtea-Web]: Enforce cache size limit.

[Deepak Bhole]

* Andrew Su <asu at redhat.com> [2011-04-19 09:46]:
> 
> 
> > 
<snip>
> 
> When I said root directory, I meant it as in the unique folder to the item.
> Such as "/home/user1/.icedtea/cache/3"
> 

Okay. That will nuke everything in the cache from existing (current
format) cache dirs, but it is okay as that cache will never get used
anyway.

> > 
> > 
> > > + long len = file.length();
> > >
> > > - if (delete || keep.contains(rStr)) {
> > > + if (delete || !file.isFile() || (maxSize >= 0 && curSize + len >
> > > maxSize) || keep.contains(rStr)) {
> > 
> > Can you clarify the logic here (in a comment as well) please?
> > 
> > 1. if delete == true, delete .... this is OK
> > 
> > 2. if !file.isFile() .... would getLRUSortedEntries() ever return a
> > dir?
> 
> No, and it shouldn't, but let's say, directory was created but the
> actual item doesn't exist. If the item wasn't marked for deletion
> we'll have an empty folder lying around, should get rid of it.
> (This seems to go with previous patch more, missed this case)
> 

Fair enough.

> > 
> > 3. (maxSize >= 0 && curSize + len > maxSize) ... okay, sounds
> > reasonable, this is current behaviour
> > 
> > 4. keep.contains(rStr) ... the else part of that block adds rStr to
> > keep. Can there be a case where it shows up again during the
> > iteration?
> 
> Nope, it doesn't the second add to keep is for use with removeUntrackedDirectories()
> Cheers,

Okay.

One last thing that just came to mind.. if I accidentally specify $HOME
as the cachedir with size < sizeof($HOME), won't this patch nuke all of it?

Deepak

>   Andrew

> diff -r 211a5e73d119 netx/net/sourceforge/jnlp/cache/CacheUtil.java
> --- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Mon Apr 18 17:38:31 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Tue Apr 19 09:33:42 2011 -0400
> @@ -533,6 +533,15 @@
>              // First we want to figure out which stuff we need to delete.
>              HashSet<String> keep = new HashSet<String>();
>              lruHandler.load();
> +            
> +            long maxSize = -1; // Default 
> +            try {
> +                maxSize = Long.parseLong(JNLPRuntime.getConfiguration().getProperty("deployment.cache.max.size"));
> +            } catch (NumberFormatException nfe) {
> +            }
> +            
> +            maxSize = maxSize << 20; // Convert from megabyte to byte (Negative values will be considered unlimited.)
> +            long curSize = 0;
>  
>              for (Entry<String, String> e : lruHandler.getLRUSortedEntries()) {
>                  // Check if the item is contained in cacheOrder.
> @@ -540,16 +549,37 @@
>                  final String value = e.getValue();
>  
>                  if (value != null) {
> +                    File file = new File(value);
>                      PropertiesFile pf = new PropertiesFile(new File(value + ".info"));
>                      boolean delete = Boolean.parseBoolean(pf.getProperty("delete"));
>  
> -                    // This will get me the root directory specific to this cache item.
> -                    String rStr = value.substring(cacheDir.length());
> +                    /*
> +                     * This will get me the root directory specific to this cache item.
> +                     * Example:
> +                     *  cacheDir = /home/user1/.icedtea/cache
> +                     *  file.getPath() = /home/user1/.icedtea/cache/0/http/www.example.com/subdir/a.jar
> +                     *  rStr first becomes: /0/http/www.example.com/subdir/a.jar
> +                     *  then rstr becomes: /home/user1/.icedtea/cache/0
> +                     */
> +                    String rStr = file.getPath().substring(cacheDir.length());
>                      rStr = cacheDir + rStr.substring(0, rStr.indexOf(File.separatorChar, 1));
> +                    long len = file.length();
>  
> -                    if (delete || keep.contains(rStr)) {
> +                    /*
> +                     * we remove entries from our lru if any of the following condition is met.
> +                     * Conditions:
> +                     *  - delete: file has been marked for deletion.
> +                     *  - !file.isFile(): if someone tampered with the directory, file doesn't exist.
> +                     *  - maxSize >= 0 && curSize + len > maxSize: If a limit was set and the new size
> +                     *  on disk would exceed the maximum size.
> +                     *  - keep.contains(rStr): We had already decided to keep an entry with the same
> +                     *  url path.
> +                     *  
> +                     */
> +                    if (delete || !file.isFile() || (maxSize >= 0 && curSize + len > maxSize) || keep.contains(rStr)) {
>                          lruHandler.removeEntry(key);
>                      } else {
> +                        curSize += len;
>                          keep.add(value.substring(rStr.length()));
>                          keep.add(rStr); // We can just use the same map, since these two things are disjoint with each other.
>                      }