Bug ID: JDK-8073187 Unexpected side effect in Pack200

Type: Bug
Component: core-libs
Sub-Component: java.util.jar

Priority: P3
Status: Closed
Resolution: Fixed

Submitted: 2015-02-16
Updated: 2016-06-13
Resolved: 2015-10-02

JDK 9
9 b85Fixed

Pack/unpack method changes default time zone and this isn't documented( at least I didn't find this):

87    public synchronized void pack(JarFile in, OutputStream out) throws IOException {
88        assert(Utils.currentInstance.get() == null);
89        TimeZone tz = (props.getBoolean(Utils.PACK_DEFAULT_TIMEZONE))
90                      ? null
91                      : TimeZone.getDefault();
92        try {
93            Utils.currentInstance.set(this);
94            if (tz != null) TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
95
96            if ("0".equals(props.getProperty(Pack200.Packer.EFFORT))) {
97                Utils.copyJarFile(in, out);
98            } else {
99                (new DoPack()).run(in, out);
100            }
101        } finally {
102            Utils.currentInstance.set(null);
103            if (tz != null) TimeZone.setDefault(tz);
104            in.close();
105        }
106    }

http://ipw83120.uk.oracle.com:8080/source/xref/jdk7u-cpu/jdk/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java#87

If we call this code concurrently, the original default time zone can be lost and set to UTC.
we've already faced with this issue in deploy code:
https://bugs.openjdk.java.net/browse/JDK-8066985

http://cr.openjdk.java.net/~ksrini/8073187/webrev.0/
23-09-2015
This bug has been marked as noreg-cleanup, with this fix the TimeZone restrictions have been removed, and the test jdk/test/tools/pack200/TimeStamp is used to verify the update and retrieval of TimeStamps in the Jar archives.
22-09-2015
Kumar, I uploaded only changes for PackerImpl.java and this leads to misunderstanding. Of course UnpackerImpl.java needs the same changes. To fix the bug we can apply this changes only to UnpackerImpl however I think it's better to keep PackerImpl and UnpackerImpl source code similar. So there's a new webrev with changes applied to both classes: http://cr.openjdk.java.net/~mcherkas/8073187/webrev.02/
04-03-2015
Reproducer is attached.
04-03-2015
Since when does the Java (Plugin) Applet pack jars ? I don't understand this, can you provide me with a reproducible test case.
24-02-2015
I will be very inconsistent to modify unpack method and leave pack as is. We have two instances of unpacker, that works in 2 threads, but unpakcer has instance level synchronization, so they can work simultaneously. Also as you remember this is allowed by java doc: http://ipw83120.uk.oracle.com:8080/source/xref/jdk7u-cpu/jdk/src/share/classes/java/util/jar/Pack200.java#126 Applet code already has been having a paralleled code for downloading and unpacking jars for long time. But looks like pack 200 ins't very popular feature and only now the race was found out by customer. The fix works fine, the customer also confirmed this.
24-02-2015
Why is this such a big problem that it needs to be fixed ? Please add your justification here.
19-02-2015
http://cr.openjdk.java.net/~mcherkas/8073187/webrev/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java.udiff.html a new version of fix, if you think that this is acceptable fix, I'll send it to review.
19-02-2015
Kumar, what do you think about this kind of fix: http://cr.openjdk.java.net/~mcherkas/8073187/webrev/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java.udiff.html It won't protect us from reading "wrong" default time zone if user code reads time zone in parallel, but it allows to use different instances of packer concurrently.
19-02-2015
The methods are all synchronized ie. so they are all mutex'ed. The trouble will be for those people who have multiple threads and a thread is performing some arbitrary TimeZone sensitive operations. The new Zip64 format allows DST I think, but we have to support the old Zip format for a long time, so that is not an option, the user has to be careful.
19-02-2015
I think we should underscore that default time zone will be changed. Because even FAQ doesn't explicitly say this. Furthermore: http://ipw83120.uk.oracle.com:8080/source/xref/jdk7u-cpu/jdk/src/share/classes/java/util/jar/Pack200.java#126 126 * <p>Note: The returned object is not guaranteed to operate 127 * correctly if multiple threads use it at the same time. 128 * A multi-threaded application should either allocate multiple 129 * packer engines, or else serialize use of one engine with a lock. it won't work correctly with multiple packer engines: Thread1 packer save DefaultTZ to local var tz Thread1 set DTZ to UTC Thread2 packer save DefaultTZ to local var tz ( wrong default tz was saved) Thread1 do packing/unpacking Thread1 . DTZ = tz Thread2 set DTZ to UTC Thread2 do packing/unpacking Thread2 DTZ = tz <--- at this point we've lost the original DTZ and set it to UTC
19-02-2015
So, it isn't just a documentation issue, currently only one packer can work in multithreaded environment, and this can lead to performance issues.
19-02-2015
I don't know how Packer200 works, does it work really depend on Default Time Zone? what about the following case: Thread1 packer save DefaultTZ to local var tz Thread2 packer save DefaultTZ to local var tz Thread1 set DTZ to UTC Thread2 set DTZ to UTC Thread1 do packing/unpacking Thread1 . return the value of default tine zone back Thread2 do packing/unpacking <---- it's working while TZ isn't UTC , is it correct? so in this case each thread has it's own packer instance, but DTZ isn't UTC while one of them work and this could lead to unexpected result.
19-02-2015
So if the documentation is to be fixed, why is this a P2 ? This is documented at: http://docs.oracle.com/javase/6/docs/technotes/guides/pack200/pack-spec.html#tocAppDes See the FAQ, http://docs.oracle.com/javase/6/docs/technotes/guides/pack200/pack-spec.html#tocAppDes but I agree this must be documented in the API documentation, I am dropping it to P4 and target this to 9. diff --git a/src/java.base/share/classes/java/util/jar/Pack200.java b/src/java.base/share/classes/java/util/jar/Pack200.java --- a/src/java.base/share/classes/java/util/jar/Pack200.java +++ b/src/java.base/share/classes/java/util/jar/Pack200.java @@ -97,7 +97,7 @@ * <p> * Unless otherwise noted, passing a <tt>null</tt> argument to a constructor or * method in this class will cause a {@link NullPointerException} to be thrown. - * + * * @author John Rose * @author Kumar Srinivasan * @since 1.5 @@ -223,6 +223,10 @@ * Note: Unless otherwise noted, passing a <tt>null</tt> argument to a * constructor or method in this class will cause a {@link NullPointerException} * to be thrown. + * <p> + * Note: The methods pack and unpack will set the TimeZone to UTC, upon + * completion the TimeZone will be reset to its original state. This + * behavior is documented in the FAQ section of JSR-200 Specification. * * @since 1.5 */
18-02-2015
Documentation is wrong: * A multi-threaded application should either allocate multiple * packer engines, or else serialize use of one engine with a lock. Our code allocates multiple engines(for each jar and each jar is processed in one thread), but this also can lead to default time zone changing. Furthermore, this side effect isn't documented, if user code works in parallel with pack200 then user code can read different default time zone depending on pack200 working phase. It may be can't be fixed in pack200, but documentation must be fixed at least.
18-02-2015
This has been documented in the API call newPacker and newUnpacker.
17-02-2015
This is how it has been since day one, ie. circa 2005. See JDK-6791805 for details, and cannot be fixed.
17-02-2015

Blocks :	JDK-8075526 - Need a way to read and write ZipEntry timestamp using local date/time without tz conversion
Duplicate :	JDK-6791805 - Pack200 unpacker implementation can set timezone to utc
Relates :	JDK-8080438 - Lack of synchronization in coding.cpp:init()
Relates :	JDK-8066985 - Java Webstart downloading packed files can result in Timezone set to UTC