Locking to protect SimpleDateFormat
SimpleDateFormat isn't thread-safe, so add locking to protect it.
Bug: 151792085
Test: atest ExifUtilsTest
diff --git a/src/com/android/providers/media/util/ExifUtils.java b/src/com/android/providers/media/util/ExifUtils.java
index 09674a8..7593ec8 100644
--- a/src/com/android/providers/media/util/ExifUtils.java
+++ b/src/com/android/providers/media/util/ExifUtils.java
@@ -32,6 +32,7 @@
import android.annotation.Nullable;
import android.media.ExifInterface;
+import androidx.annotation.GuardedBy;
import androidx.annotation.NonNull;
import java.text.ParsePosition;
@@ -49,7 +50,9 @@
// Pattern to check non zero timestamp
private static final Pattern sNonZeroTimePattern = Pattern.compile(".*[1-9].*");
+ @GuardedBy("sFormatter")
private static final SimpleDateFormat sFormatter;
+ @GuardedBy("sFormatterTz")
private static final SimpleDateFormat sFormatterTz;
static {
@@ -105,7 +108,10 @@
ParsePosition pos = new ParsePosition(0);
try {
- Date datetime = sFormatter.parse(dateTimeString, pos);
+ final Date datetime;
+ synchronized (sFormatter) {
+ datetime = sFormatter.parse(dateTimeString, pos);
+ }
if (datetime == null) return -1;
return datetime.getTime();
} catch (IllegalArgumentException e) {
@@ -122,12 +128,17 @@
try {
// The exif field is in local time. Parsing it as if it is UTC will yield time
// since 1/1/1970 local time
- Date datetime = sFormatter.parse(dateTimeString, pos);
+ Date datetime;
+ synchronized (sFormatter) {
+ datetime = sFormatter.parse(dateTimeString, pos);
+ }
if (offsetString != null) {
dateTimeString = dateTimeString + " " + offsetString;
ParsePosition position = new ParsePosition(0);
- datetime = sFormatterTz.parse(dateTimeString, position);
+ synchronized (sFormatterTz) {
+ datetime = sFormatterTz.parse(dateTimeString, position);
+ }
}
if (datetime == null) return -1;
diff --git a/tests/src/com/android/providers/media/util/ExifUtilsTest.java b/tests/src/com/android/providers/media/util/ExifUtilsTest.java
new file mode 100644
index 0000000..2b0724c
--- /dev/null
+++ b/tests/src/com/android/providers/media/util/ExifUtilsTest.java
@@ -0,0 +1,95 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.media.util;
+
+import android.media.ExifInterface;
+
+import org.junit.Test;
+
+import java.io.File;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.ToLongFunction;
+
+import static org.junit.Assert.assertEquals;
+
+public class ExifUtilsTest {
+ @Test
+ public void testParseDateTime() throws Exception {
+ final ExifInterface exif = createTestExif();
+ assertParseDateTime(exif, ExifUtils::getDateTimeOriginal);
+ }
+
+ @Test
+ public void testParseDateTimeTz() throws Exception {
+ final ExifInterface exif = createTestExif();
+ assertParseDateTime(exif, ExifUtils::getDateTimeDigitized);
+ }
+
+ @Test
+ public void testParseGpsDateTime() throws Exception {
+ final ExifInterface exif = createTestExif();
+ assertParseDateTime(exif, ExifUtils::getGpsDateTime);
+ }
+
+ private ExifInterface createTestExif() throws Exception {
+ final File file = File.createTempFile("test", ".jpg");
+ final ExifInterface exif = new ExifInterface(file);
+ exif.setAttribute(ExifInterface.TAG_DATETIME_ORIGINAL, "2016:01:28 09:17:34");
+ exif.setAttribute(ExifInterface.TAG_DATETIME_DIGITIZED, "2016:01:28 09:17:34 UTC");
+ exif.setAttribute(ExifInterface.TAG_GPS_DATESTAMP, "2016:01:28");
+ exif.setAttribute(ExifInterface.TAG_GPS_TIMESTAMP, "09:14:00");
+ return exif;
+ }
+
+ private void assertParseDateTime(ExifInterface exif, ToLongFunction<ExifInterface> func) {
+ final int numOfThreads = 5;
+ final CountDownLatch latch = new CountDownLatch(numOfThreads);
+ final AtomicInteger count = new AtomicInteger(numOfThreads);
+
+ for (int i = 0; i < numOfThreads; i++) {
+ new Thread(() -> {
+ if (parseDateTime(exif, func)) count.decrementAndGet();
+ latch.countDown();
+ }).start();
+ }
+
+ try {
+ latch.await(10, TimeUnit.SECONDS);
+ } catch (InterruptedException ignored) {
+ }
+
+ assertEquals(0, count.get());
+ }
+
+ private boolean parseDateTime(ExifInterface exif, ToLongFunction<ExifInterface> func) {
+ final long expected = func.applyAsLong(exif);
+ try {
+ for (int i = 0; i < 1000; ++i) {
+ final long actual = func.applyAsLong(exif);
+ if (expected != actual) {
+ return false;
+ }
+ }
+ } catch (ArrayIndexOutOfBoundsException | NumberFormatException e) {
+ return false;
+ }
+ return true;
+ }
+}
+