Re: RFR: 8152423: Generated temp files (+JXF...temp) for custom fonts not deleted on exit.

2016-05-13 Thread Phil Race
See updated webrev and comments at 
https://bugs.openjdk.java.net/browse/JDK-8152423


-phil.

On 05/05/2016 12:16 PM, Jim Graham wrote:
In that case, in order to avoid race conditions (GC could be 
accidentally triggered by this or subsequent shutdown hooks in the 
system), it should be released by calling disposer.dispose() instead 
of directly calling Release (still setting fontFace to null just in 
case).  That will ensure only one release per fontFace.


Also, in the constructor it looks like we'll construct a disposer for 
a null fontFace.  I don't think it hurts anything, and probably 
shouldn't happen in practice, but still seems odd...


...jim

On 05/05/2016 11:40 AM, Phil Race wrote:


On 05/05/2016 11:26 AM, Jim Graham wrote:

Is this true of any other objects managed by DWDisposer?


DWDisposer is only the class used to implement "dispose()" of
a  single DWFontFile that occurs during running/gc.

It doesn't really "manage" anything and I don't see it used
to dispose any other resource.


Would it be better to simply run through the DWDisposer queue on
shutdown and force it to dispose (i.e. release) everything it has?


There isn't a "DWDisposer" queue - it is the shared disposer queue for
all resources across Prism - so far as I can tell anyway.

This clean up on exit of font files is initiated from a shutdown
hook specific to the temporary font files (shared, not DW specific).
To be able to delete the files we need to be sure that Release() was
called before File.delete().
So we either would need the font file shutdown hook to take ownership
of first releasing other Prism resources as well or find a way to kick
off a different
hook specific to DWDisposer and ensure it runs before the deletion hook.
I am not sure there is any way to guarantee that order.

Also if we do an 8ux backport I'd want to do it this simpler way there
even if something different were done for 9.

-phil.



...jim

On 05/05/2016 11:12 AM, Phil Race wrote:

Please review :-
Bug : https://bugs.openjdk.java.net/browse/JDK-8152423
Fix : http://cr.openjdk.java.net/~prr/8152423/


"Release" was not being called on the DirectWrite font object
so it held a reference to the file, which on Windows prevents
the file from being deleted.

-phil






Re: RFR: 8152423: Generated temp files (+JXF...temp) for custom fonts not deleted on exit.

2016-05-05 Thread Jim Graham
In that case, in order to avoid race conditions (GC could be 
accidentally triggered by this or subsequent shutdown hooks in the 
system), it should be released by calling disposer.dispose() instead of 
directly calling Release (still setting fontFace to null just in case). 
 That will ensure only one release per fontFace.


Also, in the constructor it looks like we'll construct a disposer for a 
null fontFace.  I don't think it hurts anything, and probably shouldn't 
happen in practice, but still seems odd...


...jim

On 05/05/2016 11:40 AM, Phil Race wrote:


On 05/05/2016 11:26 AM, Jim Graham wrote:

Is this true of any other objects managed by DWDisposer?


DWDisposer is only the class used to implement "dispose()" of
a  single DWFontFile that occurs during running/gc.

It doesn't really "manage" anything and I don't see it used
to dispose any other resource.


Would it be better to simply run through the DWDisposer queue on
shutdown and force it to dispose (i.e. release) everything it has?


There isn't a "DWDisposer" queue - it is the shared disposer queue for
all resources across Prism - so far as I can tell anyway.

This clean up on exit of font files is initiated from a shutdown
hook specific to the temporary font files (shared, not DW specific).
To be able to delete the files we need to be sure that Release() was
called before File.delete().
So we either would need the font file shutdown hook to take ownership
of first releasing other Prism resources as well or find a way to kick
off a different
hook specific to DWDisposer and ensure it runs before the deletion hook.
I am not sure there is any way to guarantee that order.

Also if we do an 8ux backport I'd want to do it this simpler way there
even if something different were done for 9.

-phil.



...jim

On 05/05/2016 11:12 AM, Phil Race wrote:

Please review :-
Bug : https://bugs.openjdk.java.net/browse/JDK-8152423
Fix : http://cr.openjdk.java.net/~prr/8152423/


"Release" was not being called on the DirectWrite font object
so it held a reference to the file, which on Windows prevents
the file from being deleted.

-phil




Re: RFR: 8152423: Generated temp files (+JXF...temp) for custom fonts not deleted on exit.

2016-05-05 Thread Phil Race


On 05/05/2016 11:26 AM, Jim Graham wrote:

Is this true of any other objects managed by DWDisposer?


DWDisposer is only the class used to implement "dispose()" of
a  single DWFontFile that occurs during running/gc.

It doesn't really "manage" anything and I don't see it used
to dispose any other resource.

Would it be better to simply run through the DWDisposer queue on 
shutdown and force it to dispose (i.e. release) everything it has?


There isn't a "DWDisposer" queue - it is the shared disposer queue for
all resources across Prism - so far as I can tell anyway.

This clean up on exit of font files is initiated from a shutdown
hook specific to the temporary font files (shared, not DW specific).
To be able to delete the files we need to be sure that Release() was
called before File.delete().
So we either would need the font file shutdown hook to take ownership
of first releasing other Prism resources as well or find a way to kick 
off a different

hook specific to DWDisposer and ensure it runs before the deletion hook.
I am not sure there is any way to guarantee that order.

Also if we do an 8ux backport I'd want to do it this simpler way there
even if something different were done for 9.

-phil.



...jim

On 05/05/2016 11:12 AM, Phil Race wrote:

Please review :-
Bug : https://bugs.openjdk.java.net/browse/JDK-8152423
Fix : http://cr.openjdk.java.net/~prr/8152423/


"Release" was not being called on the DirectWrite font object
so it held a reference to the file, which on Windows prevents
the file from being deleted.

-phil




Re: RFR: 8152423: Generated temp files (+JXF...temp) for custom fonts not deleted on exit.

2016-05-05 Thread Jim Graham
Is this true of any other objects managed by DWDisposer?  Would it be 
better to simply run through the DWDisposer queue on shutdown and force 
it to dispose (i.e. release) everything it has?


...jim

On 05/05/2016 11:12 AM, Phil Race wrote:

Please review :-
Bug : https://bugs.openjdk.java.net/browse/JDK-8152423
Fix : http://cr.openjdk.java.net/~prr/8152423/


"Release" was not being called on the DirectWrite font object
so it held a reference to the file, which on Windows prevents
the file from being deleted.

-phil


RFR: 8152423: Generated temp files (+JXF...temp) for custom fonts not deleted on exit.

2016-05-05 Thread Phil Race

Please review :-
Bug : https://bugs.openjdk.java.net/browse/JDK-8152423
Fix : http://cr.openjdk.java.net/~prr/8152423/ 



"Release" was not being called on the DirectWrite font object
so it held a reference to the file, which on Windows prevents
the file from being deleted.

-phil