From 1ed577545fd1c9802da5a17fd350bacb21820085 Mon Sep 17 00:00:00 2001 From: Sergey Matvienko Date: Tue, 4 Jul 2023 15:25:05 +0200 Subject: [PATCH 1/3] TbDate toISOString sync as required SimpleDateFormat --- .../src/main/java/org/thingsboard/script/api/tbel/TbDate.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java b/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java index 9c360e5c7e..d8c5fb2265 100644 --- a/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java +++ b/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java @@ -73,7 +73,9 @@ public class TbDate extends Date { } public String toISOString() { - return isoDateFormat.format(this); + synchronized (TbDate.class) { + return isoDateFormat.format(this); + } } public String toLocaleString(String locale) { From 235c6f94d907afcc4aa483026e9a2e3aea67aa63 Mon Sep 17 00:00:00 2001 From: Sergey Matvienko Date: Wed, 12 Jul 2023 18:29:02 +0200 Subject: [PATCH 2/3] TbDateTest added --- .../script/api/tbel/TbDateTest.java | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java diff --git a/common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java b/common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java new file mode 100644 index 0000000000..11d65e0ba7 --- /dev/null +++ b/common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java @@ -0,0 +1,102 @@ +/** + * Copyright © 2016-2023 The Thingsboard Authors + * + * 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 org.thingsboard.script.api.tbel; + +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +@Slf4j +class TbDateTest { + + ListeningExecutorService executor; + + @AfterEach + void tearDown() { + if (executor != null) { + executor.shutdownNow(); + } + } + + /** + * Note: This test simulated the high concurrency calls. + * But it not always fails before as the concurrency issue happens far away from the test subject, inside the SimpleDateFormat class. + * Few calls later after latch open, the concurrency issue is not well reproduced. + * Depends on environment some failure may happen each 2 or 100 runs. + * To be highly confident run this test in a ForkMode=method, repeat 100 times. This will provide about 99 failures per 100 runs. + * The value of this test is *never* to fail when isoDateFormat.format(this) is properly synchronized + * If this test fails time-to-time -- it is a sign that isoDateFormat.format() called concurrently and have to be fixed (synchronized) + * The expected exception example: + * Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 14 out of bounds for length 13 + * at java.base/sun.util.calendar.BaseCalendar.getCalendarDateFromFixedDate(BaseCalendar.java:457) + * at java.base/java.util.GregorianCalendar.computeFields(GregorianCalendar.java:2394) + * at java.base/java.util.GregorianCalendar.computeFields(GregorianCalendar.java:2309) + * at java.base/java.util.Calendar.setTimeInMillis(Calendar.java:1834) + * at java.base/java.util.Calendar.setTime(Calendar.java:1800) + * at java.base/java.text.SimpleDateFormat.format(SimpleDateFormat.java:974) + */ + @Test + void testToISOStringConcurrently() throws ExecutionException, InterruptedException, TimeoutException { + int threads = 5; + executor = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(threads)); + for (int j = 0; j < 1000; j++) { + final int iteration = j; + CountDownLatch readyLatch = new CountDownLatch(threads); + CountDownLatch latch = new CountDownLatch(1); + long now = 1709217342000L; + List> futures = new ArrayList<>(threads); + for (int i = 0; i < threads; i++) { + long ts = now + TimeUnit.DAYS.toMillis(i * 366) + TimeUnit.MINUTES.toMillis(iteration) + TimeUnit.SECONDS.toMillis(iteration) + iteration; + TbDate tbDate = new TbDate(ts); + futures.add(executor.submit(() -> { + readyLatch.countDown(); + if (!latch.await(30, TimeUnit.SECONDS)) { + throw new RuntimeException("await timeout"); + } + return tbDate.toISOString(); + })); + } + ListenableFuture> future = Futures.allAsList(futures); + Futures.addCallback(future, new FutureCallback>() { + @Override + public void onSuccess(List result) { + + } + + @Override + public void onFailure(Throwable t) { + log.error("Failure happens on iteration {}", iteration); + } + }, MoreExecutors.directExecutor()); + readyLatch.await(30, TimeUnit.SECONDS); + latch.countDown(); + future.get(30, TimeUnit.SECONDS); + } + } +} \ No newline at end of file From 2a683f9977b4acb5bd9551406e7f0f4e688c46a3 Mon Sep 17 00:00:00 2001 From: Sergey Matvienko Date: Tue, 18 Jul 2023 20:24:31 +0200 Subject: [PATCH 3/3] TbDate isoDateFormat is ThreadLocal instead synchronization --- .../thingsboard/script/api/tbel/TbDate.java | 7 +++-- .../script/api/tbel/TbDateTest.java | 27 ++++++++++++++++++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java b/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java index d8c5fb2265..c22e2c329d 100644 --- a/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java +++ b/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/TbDate.java @@ -35,7 +35,8 @@ public class TbDate extends Date { private static final DateTimeFormatter isoDateFormatter = DateTimeFormatter.ofPattern( "yyyy-MM-dd[[ ]['T']HH:mm[:ss[.SSS]][ ][XXX][Z][z][VV][O]]").withZone(ZoneId.systemDefault()); - private static final DateFormat isoDateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); + private static final ThreadLocal isoDateFormat = ThreadLocal.withInitial(() -> + new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ")); public TbDate() { super(); @@ -73,9 +74,7 @@ public class TbDate extends Date { } public String toISOString() { - synchronized (TbDate.class) { - return isoDateFormat.format(this); - } + return isoDateFormat.get().format(this); } public String toLocaleString(String locale) { diff --git a/common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java b/common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java index 11d65e0ba7..88f1f825c1 100644 --- a/common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java +++ b/common/script/script-api/src/test/java/org/thingsboard/script/api/tbel/TbDateTest.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import java.util.ArrayList; +import java.util.Calendar; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; @@ -32,6 +33,8 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import static org.assertj.core.api.Assertions.assertThat; + @Slf4j class TbDateTest { @@ -99,4 +102,26 @@ class TbDateTest { future.get(30, TimeUnit.SECONDS); } } -} \ No newline at end of file + + @Test + void testToISOStringThreadLocalStaticFormatter() throws ExecutionException, InterruptedException, TimeoutException { + executor = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(1)); + long ts = 1709217342987L; //Thu Feb 29 2024 14:35:42.987 GMT+0000 + int offset = Calendar.getInstance().get(Calendar.ZONE_OFFSET); // for example 3600000 for GMT + 1 + TbDate tbDate = new TbDate(ts - offset); + String datePrefix = "2024-02-29T14:35:42.987"; //without time zone + assertThat(tbDate.toISOString()) + .as("format in main thread") + .startsWith(datePrefix); + assertThat(executor.submit(tbDate::toISOString).get(30, TimeUnit.SECONDS)) + .as("format in executor thread") + .startsWith(datePrefix); + assertThat(new TbDate(ts - offset).toISOString()) + .as("new instance format in main thread") + .startsWith(datePrefix); + assertThat(executor.submit(() -> new TbDate(ts - offset).toISOString()).get(30, TimeUnit.SECONDS)) + .as("new instance format in executor thread") + .startsWith(datePrefix); + } + +}