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

[Deepak Bhole]

* Denis Lila <dlila at redhat.com> [2011-04-20 10:31]:
> > What discussions? Please post this kind of stuff to the list..
> 
> Right, sorry about that.
> 
> Well, basically the sets remove and keep were disjoint, so
> remove.removeAll(keep) should be eliminated.
> 
> Also, we were adding rStr to keep, but checking for
> file.getPath().substring(rStr.length()) which
> would always be false.
> 
> With these changes, the patch looks good to me.
> 

Likewise. OK from me too.

Deepak

> Regards,
> Denis.
> 
> ----- Original Message -----
> > * Andrew Su <asu at redhat.com> [2011-04-20 10:25]:
> > >
> > >
> > > ----- Original Message -----
> > > > From: "Deepak Bhole" <dbhole at redhat.com>
> > > > To: "Andrew Su" <asu at redhat.com>
> > > > Cc: "IcedTea" <distro-pkg-dev at openjdk.java.net>
> > > > Sent: Tuesday, April 19, 2011 6:29:01 PM
> > > > Subject: Re: [RFC][Icedtea-Web]: Enforce cache size limit.
> > > > * Andrew Su <asu at redhat.com> [2011-04-19 17:30]:
> > > > >
> > > > >
> > > > > ----- Original Message -----
> > > > <snip>
> > > > > + } catch (IOException e1) {
> > > > > + e1.printStackTrace();
> > > >
> > > >
> > > > Why e1 and not e?
> > >
> > > e is being used for iterating through the entries already.
> > >
> > > I've attached an updated patch with some changes discussed on irc
> > > with Denis.
> > >
> > 
> 
> > 
> > Thanks,
> > Deepak
> > 
> > > >
> > > > Rest looks fine to me. Okay from me after above e1 -> e change.
> > > >
> > > > Cheers,
> > > > Deepak
> > > >
> > > > > + }
> > > > > + }
> > > > > + }
> > > > >                      }
> > > > >                  } else {
> > > > >                      lruHandler.removeEntry(key);
> > > > > @@ -560,20 +600,26 @@
> > > > >              }
> > > > >              lruHandler.store();
> > > > >
> > > > > - removeUntrackedDirectories(keep);
> > > > > + /*
> > > > > + * FIXME: if cacheDir is for example $USER_HOME and they have a
> > > > > folder called http
> > > > > + * and/or https. These would get removed.
> > > > > + */
> > > > > + remove.add(cacheDir + File.separator + "http");
> > > > > + remove.add(cacheDir + File.separator + "https");
> > > > > +
> > > > > + remove.removeAll(keep);
> > > > > +
> > > > > + removeSetOfDirectories(remove);
> > > > > +
> > > > >          }
> > > > >      }
> > > > >
> > > > > - private static void removeUntrackedDirectories(Set<String>
> > > > > keep) {
> > > > > - File temp = new File(cacheDir);
> > > > > - // Remove all folder not listed in keep.
> > > > > - for (File f : temp.listFiles()) {
> > > > > - if (f.isDirectory() && !keep.contains(f.getPath())) {
> > > > > - try {
> > > > > - FileUtils.recursiveDelete(f, f);
> > > > > - } catch (IOException e) {
> > > > > - e.printStackTrace();
> > > > > - }
> > > > > + private static void removeSetOfDirectories(Set<String> remove)
> > > > > {
> > > > > + for (String s : remove) {
> > > > > + File f = new File(s);
> > > > > + try {
> > > > > + FileUtils.recursiveDelete(f, f);
> > > > > + } catch (IOException e) {
> > > > >              }
> > > > >          }
> > > > >      }
> > 
> > > diff -r bc48727cfc1a netx/net/sourceforge/jnlp/cache/CacheUtil.java
> > > --- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java Wed Apr 20
> > > 09:26:50 2011 -0400
> > > +++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java Wed Apr 20
> > > 10:24:52 2011 -0400
> > > @@ -533,7 +533,17 @@
> > >          if (okToClearCache()) {
> > >              // First we want to figure out which stuff we need to
> > >              delete.
> > >              HashSet<String> keep = new HashSet<String>();
> > > + HashSet<String> remove = 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.
> > > @@ -541,18 +551,49 @@
> > >                  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(...): We had already decided to keep an entry
> > > with the same
> > > + * url path.
> > > + */
> > > + if (delete || !file.isFile() || (maxSize >= 0 && curSize + len >
> > > maxSize) ||
> > > + keep.contains(file.getPath().substring(rStr.length()))) {
> > >                          lruHandler.removeEntry(key);
> > > + remove.add(rStr);
> > >                      } else {
> > > - 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.
> > > + curSize += len;
> > > + keep.add(file.getPath().substring(rStr.length()));
> > > +
> > > + for (File f : file.getParentFile().listFiles()) {
> > > + if (!(f.equals(file) || f.equals(pf.getStoreFile()))){
> > > + try {
> > > + FileUtils.recursiveDelete(f, f);
> > > + } catch (IOException e1) {
> > > + e1.printStackTrace();
> > > + }
> > > + }
> > > + }
> > >                      }
> > >                  } else {
> > >                      lruHandler.removeEntry(key);
> > > @@ -560,20 +601,24 @@
> > >              }
> > >              lruHandler.store();
> > >
> > > - removeUntrackedDirectories(keep);
> > > + /*
> > > + * FIXME: if cacheDir is for example $USER_HOME and they have a
> > > folder called http
> > > + * and/or https. These would get removed.
> > > + */
> > > + remove.add(cacheDir + File.separator + "http");
> > > + remove.add(cacheDir + File.separator + "https");
> > > +
> > > + removeSetOfDirectories(remove);
> > > +
> > >          }
> > >      }
> > >
> > > - private static void removeUntrackedDirectories(Set<String> keep) {
> > > - File temp = new File(cacheDir);
> > > - // Remove all folder not listed in keep.
> > > - for (File f : temp.listFiles()) {
> > > - if (f.isDirectory() && !keep.contains(f.getPath())) {
> > > - try {
> > > - FileUtils.recursiveDelete(f, f);
> > > - } catch (IOException e) {
> > > - e.printStackTrace();
> > > - }
> > > + private static void removeSetOfDirectories(Set<String> remove) {
> > > + for (String s : remove) {
> > > + File f = new File(s);
> > > + try {
> > > + FileUtils.recursiveDelete(f, f);
> > > + } catch (IOException e) {
> > >              }
> > >          }
> > >      }