Created attachment 60952 [details]
screenshot

Fix of this bug requires one of the following:

a) make the JDK developers fix their bug #4744247 (i.e. do not fix this, just wait until the bug is fixed)
b) make an ugly hack in the search routine such that decoding of .properties files is done in a special way

Fixed.

I made a bridge module between "Resource Bundles" (properties) and "User Utilities" (utilities), named "Searching in
Properties Files" (properties.search). This modules defines a special routine for obtaining CharsetDecoder for a given
file. The routine is special in that it calls a two-argument constructor of PropertiesEncoding.PropCharsetDecoder. The
second argument passes information about length of the file that is about to be decoded such that the decoder knows when
to flush the buffer, allowing it to workaround the JDK bug.

Changeset Id:
3b6c198e12df
(http://hg.netbeans.org/main/rev/3b6c198e12df)

Wouldn't it have sufficed for PropertiesEncoding.getEncoding(FileObject) to return a (Prop)Charset which retained the
file length as an instance field, passing it in turn to (Prop)CharsetDecoder?

It would be a good solution if there was a guarantee that the encoding (Charset) provided by method
PropertiesEncoding.getEncoding(FileObject) is always used immediately after it is obtained. To be accurate, the
condition is that the FileObject's length must not change since the Charset is obtained until it is used. I can only
guarantee this for the Utilities module, not for other modules.

Perhaps the PropCharset could retain the FileObject itself as an instance field, only checking the file's length when
actual decoding begins.

Yes, it should work. And the reference to the FileObject could be weak.

Yes you could use a weak reference, though I cannot think of a situation where you would hold on to the Charset of a
file without wanting to holding on to the file itself.

"... I cannot think of a situation where you would hold on to the Charset of a file without wanting to holding on to the
file itself."

That is also the reason why it is safe to use weak references. If such situation happens (by accident), method
newDecoder() will throw an IllegalStateException("trying to use a decoder for a non-existing FileObject"). But it may
happen that someone keeps a reference to the PropCharset even though it is not necessary any more. In this case, the
weak reference will not prevent the FileObject from being garbage-collected.

If I understand correctly, you would like me to remove the bridge module and use the described solution instead, right?

True, and you could even keep both

private long size;
private /*Weak*/Reference<FileObject> file;

to cover all possibilities.

If the implementation I suggested really works, then of course it would be nicer - just much simpler code, one less
module, etc.

Why keep the "size" field? When and how could I use it?

In case the FileObject was collected, then you could use the stored size rather than throwing ISE. (The file may well
still exist on disk - the FO can be collected and then recreated.)

I would prefer not using the cached size. If the FileObject was garbage-collected during the period since initialization
of the Charset until the decoding begins and the corresponding file gets smaller during that period, the decoder might
drop some bytes from input. I believe this is a real danger.

Even if the ISE is called, the same situation might happen during the period since initialization of the decoder, but I
consider this much less probable.

You could keep a URL for the FileObject. This poses no memory leak threat.

That is a good idea.

So I will use a combination of URL and a weak reference to a FileObject. The weak reference will be used in the first
place. If the FileObject cannot be obtained from the weak reference, the URLMapper.findFileObject(URL) will be used. If
it fails, too, then an ISE will be thrown. All of this will happen in method PropCharset.newDecoder().

I realize that to make it correct to the maximum extent, I could attach a FileChangeListener and update the URL each
time the FileObject is renamed. But I am not sure it is worth doing it.

I have reworked the fix as explained in the above discussion. The bridge module is removed.

Changeset Id:
49ebdda2090e
(http://hg.netbeans.org/main/rev/49ebdda2090e)

Just reopening for an uncompilable test:

http://deadlock.netbeans.org/hudson/job/test-compilation/975/testReport/org.netbeans.nbbuild/SubAntJUnitReport/_hudson_workdir_jobs_test_compilation_workspace_properties/

/hudson/workdir/jobs/test-compilation/workspace/properties/test/unit/src/org/netbeans/modules/properties/PropertiesEncodingTest.java:63:
PropCharset() has private access in org.netbeans.modules.properties.PropertiesEncoding.PropCharset
                = new PropCharsetEncoder(new PropCharset());
                                         ^

Fixed at Jul 28 18:20 +0000 2008.

Integrated into 'main-golden', available in NB_Trunk_Production #352 build
Changeset: http://hg.netbeans.org/main/rev/49ebdda2090e
User: Marian Petras <mpetras@netbeans.org>
Log: simplified fix of bug #134371 - now without any bridge module

does this still depend on the jdk issue being fixed also or is this fix the workaround
discussed to not need that issue to be fixed ?


and where in ide does this fix apply to, since 123631 was just about editor ?

ken.frank@sun.com

The change that introduced the bridge module (changeset 3b6c198e12df) added a workaround for searching files.

The subsequent change (changeset 49ebdda2090e) simplified the fix (also removed the bridge module) and made it work
across the IDE such that there is no need for special patches for the editor, searching etc. It should be working
correctly in 99.99% of cases but still it might happen that it would not work in some rare cases. Of course, the fix in
JDK would eliminate the need for the workaround, resulting in 100% reliability and reduced complexity.

v