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
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 b85Fixed
Related Reports
Blocks :  
Duplicate :  
Relates :  
Relates :  
Description
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
Comments
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