RE: [cp-patches] Changes to ThreadLocal

2007-08-22 Thread Jeroen Frijters
Ian Rogers wrote:
> Thanks Jeroen, it's clear now. The problem with a phantom reference is
> that when we collect the thread local we should also really make
> collectable all of the values set to thread locals. A phantom reference
> won't do this and so introduces a memory leak until something can run
> over the queue of phantom references freeing all the allocated thread
> local slots. As you say, we can use phantom references, wouldn't
> another way be to implement say our own system weak reference, with a
> guarantee there are no references and weak references to the object?
> Having a thread that polls a normally empty queue seems undesirable
> both for this and Classpath bug 29499.

The thread doesn't actually need to poll (ReferenceQueue.remove() is a blocking 
operation), but burning a thread on this does suck. OTOH, there are a lot of 
other areas in the class lib that create background threads.

I think the best thing would be to have a VM interface that handles the clean 
up and provide a default implementation that uses PhantomReference and allows 
VMs to provide a more efficient clean up mechanism. Ideally the mechanism would 
be flexible enough to also be used for #29499.

OpenJDK has the sun.misc.Cleaner class for this. It extends PhantomReference, 
but it is treated specially by the finalizer thread, so that these aren't 
actually enqueued but processed by the finalizer thread itself.

Regards,
Jeroen




Re: [cp-patches] Changes to ThreadLocal

2007-08-22 Thread Ian Rogers

Jeroen Frijters wrote:

No, any VM that implements the spec is vulnerable. Here's an example of a 
possible attack:

class Attacker
{
  static ArrayList resurrected;
  ArrayList list = new ArrayList();

  Attacker()
  {
for (int i = 0; i < 1000; i++)
{
  list.add(new ThreadLocal());
}
  }

  protected void finalize()
  {
resurrected = list;
  }

  static void attack()
  {
new Attacker();
System.gc();
System.runFinalization();
runSomeHighTrustCodeThatWillAllocateThreadLocal();
for (ThreadLocal t : resurrected)
{
   if (t.get() != null)
   {
  System.out.println("We've stolen the object:" + t.get());
   }
}
  }
}

I hope this clarifies the problem with finalizers. A PhantomReference doesn't 
suffer from this problem, because it only gets enqueued after the object 
*really* is gone. For another example of this bug in GNU Classpath see 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29499
  


Thanks Jeroen, it's clear now. The problem with a phantom reference is 
that when we collect the thread local we should also really make 
collectable all of the values set to thread locals. A phantom reference 
won't do this and so introduces a memory leak until something can run 
over the queue of phantom references freeing all the allocated thread 
local slots. As you say, we can use phantom references, wouldn't another 
way be to implement say our own system weak reference, with a guarantee 
there are no references and weak references to the object? Having a 
thread that polls a normally empty queue seems undesirable both for this 
and Classpath bug 29499.


Thanks again,
Ian