1.0.1 rc1 #8
Closed
1.0.1 rc1 #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…NAPSHOT. The tag 1.0.0 targets primefaces 3.5, which the version we need to make sure that works for the time being.
…mefaces 3.5 version the fixes to the web-fragment.xml done on the master branch; the temp file leak bug that existed in the GraphicImageManager where it was always registering a new empty image on each refresh, and an additional bug that probably was fixed on release 4.0 but not on release for 1.0.0 where the uniqueId produced by the renderer would always be different on each request. This bug was identical to the GraphicImageManager bug, in the sense that on the first visit to the page the dynamic image would be properly rendered, but on subsequent requests a new Id would be produced creating a file leakage and finally the file would be empty of content because the input stream had been closed already
…file with potentially empty cotent, we should also ensure that the we do not keep feeding out a tmp file with stale data within it. This a real possibility depending on how unique the uniqueId really is. On this checking the registerImage algorithm is modified to take into account weather or not there are any available bytes to write to an output file. If there are, we do ahead and we write a new tmp file.
…ed algorithm should be used. If we explictely told the algorithm to be used the code would break when for example the StreamedContent would be null. With the current changes the algorithm will only run if it has a chance to succeed. Fixed the problem correlated with uniqueIds. It is now clear why the original implementation was always saving new images. The original iplementation did this because - browsers are chaching images so if we update the content but do not change the reference, the browser will not download a new image. Therefore, work-around the implementation to conciliate both needs. One the need to not save a new image and not produce a new uniqueId when the StreamedContent has been closed and trying to save a new image fill just leak files of 0 bytes. Two, produce a new uniqueId whenever the stream content does hava data available and writing an image has a chance to succeed.
…for provided uniqueId. This happened because the browser had chached an image for a uniqueId reference and on server startup the uniqueId was again the same as before startup only the image uploaded was different. This is fixed by adding to the ImageManager a random sessionid value that is fixed for a given session but constant throughout the lifetime of a sesison. We keep producing uniqueIds with a consitent pattern that will ensure we bypass the browser cache but continue being smart enough to delete stale images. In addition fixed a bug in the imageResourceHandler where an InputStream for a stream content was not being closed and this was leading to FileHandlers being held open by the JVM for a long period of time until garbage collection took over to clean the file handlers. The not closing of the stream was making the delete mechanism of the resource manager not work consistently.
…ion scoped and access to the satate should anyway be done with synchronization on the public apis. Put the class as implementation java.io.serializable, otherwise if the container decides to add a reference to the bean to session map it will fail with class no serializable exception
|
There is something wrong with the pull request, as it says it is to be a pull request to the master. It should have been targeting a the 1.0.0 branch actually. |
|
I don't have the time to review this for the moment, but I'll come back to it within a few weeks. I will verify the changes against the Application on Tomcat where it was originally derived from. Thx for the input |
|
Hi No problem, Until then, I will try to see if I find the time to merge my changes upwards to my local forked master branch, which should make it easier for you if you decide to merge it or only parts of it. For the time being, I will be using the 1.0.1-RC1 as it works around the view access scoped limitation in a particular scenario of our application under primefaces 3.5 with delta spike. Kindest regards. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
I would like you to consider the changes i've checked in to the 1.0.1 branch.
Please, I've tested it under welogic 12.1.2 and glassfish 3.1.2.9 both JEE 6 containers.
I would appreciate if you could review the branch and hopefully take it over.
The branches has:
(1) Synchronization that was missing on the resource manager - very simple synchroniztaiton
(2) Buts the Resource Manager as serilizable
(3) Reviews the process of generate unique ids - not even in the HEAD release is the code reselient to page refresh leaking images of 0 bytes due to the stream being closed.
(4) Is reselient to browser cache by producing a new unique id in a pseudo deterministic way, meaning that when it makes sense to produce a new unique id a new unique id is produced and it will be an id that the browser has not yet sean safeguarding from cache issues
(5) fixes leaking file input sream on the resource handler tha twas not closing the stream
(6) is more light weith in terms of pyhsical space consumption in the temp folder as it attempts on the fly to delete the stale old image generated on a previous request when a new stream content is created for the renderer
Kindest regards,
Nuno.