archive/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2025-09-15 08:34:30 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

8191216: SimpleTimeZone.clone() has a data race on cache fields

Browse source 
Reviewed-by: alanb, naoto
This commit is contained in:
Peter Levart 2017-12-12 00:30:57 +01:00
parent 3b0e59e8d8
commit 238ca2e781
2 changed files with 173 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

76
src/java.base/share/classes/java/util/SimpleTimeZone.java
View file

@ -548,12 +548,11 @@ public class SimpleTimeZone extends TimeZone {
computeOffset:
if (useDaylight) {
synchronized (this) {
if (cacheStart != 0) {
if (date >= cacheStart && date < cacheEnd) {
offset += dstSavings;
break computeOffset;
}
Cache cache = this.cache;
if (cache != null) {
if (date >= cache.start && date < cache.end) {
offset += dstSavings;
break computeOffset;
}
}
BaseCalendar cal = date >= GregorianCalendar.DEFAULT_GREGORIAN_CUTOVER ?
@ -671,14 +670,13 @@ public class SimpleTimeZone extends TimeZone {
}
private int getOffset(BaseCalendar cal, BaseCalendar.Date cdate, int year, long time) {
synchronized (this) {
if (cacheStart != 0) {
if (time >= cacheStart && time < cacheEnd) {
return rawOffset + dstSavings;
}
if (year == cacheYear) {
return rawOffset;
}
Cache cache = this.cache;
if (cache != null) {
if (time >= cache.start && time < cache.end) {
return rawOffset + dstSavings;
}
if (year == cache.year) {
return rawOffset;
}
}
@ -689,11 +687,7 @@ public class SimpleTimeZone extends TimeZone {
if (time >= start && time < end) {
offset += dstSavings;
}
synchronized (this) {
cacheYear = year;
cacheStart = start;
cacheEnd = end;
}
this.cache = new Cache(year, start, end);
} else {
if (time < end) {
// TODO: support Gregorian cutover. The previous year
@ -711,12 +705,7 @@ public class SimpleTimeZone extends TimeZone {
}
}
if (start <= end) {
synchronized (this) {
// The start and end transitions are in multiple years.
cacheYear = (long) startYear - 1;
cacheStart = start;
cacheEnd = end;
}
this.cache = new Cache((long) startYear - 1, start, end);
}
}
return offset;
@ -876,7 +865,7 @@ public class SimpleTimeZone extends TimeZone {
* Generates the hash code for the SimpleDateFormat object.
* @return the hash code for this object
*/
public synchronized int hashCode()
public int hashCode()
{
return startMonth ^ startDay ^ startDayOfWeek ^ startTime ^
endMonth ^ endDay ^ endDayOfWeek ^ endTime ^ rawOffset;
@ -1201,19 +1190,27 @@ public class SimpleTimeZone extends TimeZone {
/**
* Cache values representing a single period of daylight saving
* time. When the cache values are valid, cacheStart is the start
* time (inclusive) of daylight saving time and cacheEnd is the
* end time (exclusive).
* time. Cache.start is the start time (inclusive) of daylight
* saving time and Cache.end is the end time (exclusive).
*
* cacheYear has a year value if both cacheStart and cacheEnd are
* in the same year. cacheYear is set to startYear - 1 if
* cacheStart and cacheEnd are in different years. cacheStart is 0
* if the cache values are void. cacheYear is a long to support
* Integer.MIN_VALUE - 1 (JCK requirement).
* Cache.year has a year value if both Cache.start and Cache.end are
* in the same year. Cache.year is set to startYear - 1 if
* Cache.start and Cache.end are in different years.
* Cache.year is a long to support Integer.MIN_VALUE - 1 (JCK requirement).
*/
private transient long cacheYear;
private transient long cacheStart;
private transient long cacheEnd;
private static final class Cache {
final long year;
final long start;
final long end;
Cache(long year, long start, long end) {
this.year = year;
this.start = start;
this.end = end;
}
}
private transient volatile Cache cache;
/**
* Constants specifying values of startMode and endMode.
@ -1282,9 +1279,8 @@ public class SimpleTimeZone extends TimeZone {
// Maximum number of rules.
private static final int MAX_RULE_NUM = 6;
private synchronized void invalidateCache() {
cacheYear = startYear - 1;
cacheStart = cacheEnd = 0;
private void invalidateCache() {
cache = null;
}
//----------------------------------------------------------------------

137
test/jdk/java/util/TimeZone/SimpleTimeZoneCloneRaceTest.java Normal file
View file

@ -0,0 +1,137 @@
/*
* Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
import java.util.Calendar;
import java.util.Locale;
import java.util.SimpleTimeZone;
import java.util.TimeZone;
import java.util.function.Supplier;
/*
* @test
* @bug 8191216
* @summary test that provokes race between cloning and lazily initializing
* SimpleTimeZone cache fields
*/
public class SimpleTimeZoneCloneRaceTest {
public static void main(String[] args) throws InterruptedException {
// shared TZ user repeatedly samples sharedTZ and calculates offset
// using the shared instance
TimeZoneUser sharedTZuser = new TimeZoneUser(() -> sharedTZ);
// cloned TZ user repeatedly samples sharedTZ then clones it and
// calculates offset using the clone...
TimeZoneUser clonedTZuser = new TimeZoneUser(() -> {
// sample shared TZ
TimeZone tz = sharedTZ;
// do some computation that takes roughly the same time as it takes
// sharedTZUser to start changing cache fields in shared TZ
cpuHogTZ.getOffset(time);
// now clone the sampled TZ and return it, hoping the clone is done
// at about the right time....
return (TimeZone) tz.clone();
});
// start threads
Thread t1 = new Thread(sharedTZuser);
Thread t2 = new Thread(clonedTZuser);
t1.start();
t2.start();
// plant new SimpleTimeZone instances for 2 seconds
long t0 = System.currentTimeMillis();
do {
TimeZone tz1 = createSTZ();
TimeZone tz2 = createSTZ();
cpuHogTZ = tz1;
sharedTZ = tz2;
} while (System.currentTimeMillis() - t0 < 2000L);
sharedTZuser.stop = true;
clonedTZuser.stop = true;
t1.join();
t2.join();
System.out.println(
String.format("shared TZ: %d correct, %d incorrect calculations",
sharedTZuser.correctCount, sharedTZuser.incorrectCount)
);
System.out.println(
String.format("cloned TZ: %d correct, %d incorrect calculations",
clonedTZuser.correctCount, clonedTZuser.incorrectCount)
);
if (clonedTZuser.incorrectCount > 0) {
throw new RuntimeException(clonedTZuser.incorrectCount +
" fatal data races detected");
}
}
static SimpleTimeZone createSTZ() {
return new SimpleTimeZone(-28800000,
"America/Los_Angeles",
Calendar.APRIL, 1, -Calendar.SUNDAY,
7200000,
Calendar.OCTOBER, -1, Calendar.SUNDAY,
7200000,
3600000);
}
static volatile TimeZone cpuHogTZ = createSTZ();
static volatile TimeZone sharedTZ = createSTZ();
static final long time;
static final long correctOffset;
static {
TimeZone tz = createSTZ();
Calendar cal = Calendar.getInstance(tz, Locale.ROOT);
cal.set(2000, Calendar.MAY, 1, 0, 0, 0);
time = cal.getTimeInMillis();
correctOffset = tz.getOffset(time);
}
static class TimeZoneUser implements Runnable {
private final Supplier<? extends TimeZone> tzSupplier;
TimeZoneUser(Supplier<? extends TimeZone> tzSupplier) {
this.tzSupplier = tzSupplier;
}
volatile boolean stop;
int correctCount, incorrectCount;
@Override
public void run() {
while (!stop) {
TimeZone tz = tzSupplier.get();
int offset = tz.getOffset(time);
if (offset == correctOffset) {
correctCount++;
} else {
incorrectCount++;
}
}
}
}
}