ZK-5611: Executions.schedule() might fail if it's called by multiple …

…threads

zk/src/main/java/org/zkoss/zk/ui/impl/DesktopImpl.java

@@ -1626,14 +1626,17 @@ public <T extends Event> void scheduleServerPush(EventListener<T> listener, T ev
 			public void schedule(EventListener<T> listener, T event) {
 				if (log.isDebugEnabled()) {
 					log.debug("scheduleServerPush: [{}]", event);
-
 				}
-				if (!scheduledServerPush()) {
-					// reset write/read count
-					_scheduleInfoReadCount.set(0);
-					_scheduleInfoWriteCount.set(0);
+
+				synchronized (_schedInfos) {
+					if (!scheduledServerPush()) {
+						// reset write/read count
+						_scheduleInfoReadCount.set(0);
+						_scheduleInfoWriteCount.set(0);
+					}
+					_schedInfos.put(_scheduleInfoWriteCount.getAndIncrement(),
+							new ScheduleInfo<T>(listener, event));
 				}
-				_schedInfos.put(_scheduleInfoWriteCount.getAndIncrement(), new ScheduleInfo<T>(listener, event));
 			}
 		});
 	}
@@ -1834,19 +1837,23 @@ public void onEvent(Event event) throws Exception {
 			}
 			while (scheduledServerPush()) {
 				final Integer key = _scheduleInfoReadCount.getAndIncrement();
-				ScheduleInfo<? extends Event> ifPresent = _schedInfos.getIfPresent(key);
+				ScheduleInfo<? extends Event> ifPresent = _schedInfos.getIfPresent(
+						key);
 				if (ifPresent != null) {
 					ifPresent.invoke();
 					_schedInfos.invalidate(key);
 				} else if (scheduledServerPush()) {
-					// just in case to avoid endless loop
-					List<Integer> keys = new ArrayList<>(_schedInfos.asMap().keySet());
-					if (!keys.isEmpty()) {
-						Collections.sort(keys);
-						_scheduleInfoReadCount.set(keys.get(0));
-						continue;
-					} else {
-						break;
+					synchronized (_schedInfos) {
+						// just in case to avoid endless loop
+						List<Integer> keys = new ArrayList<>(
+								_schedInfos.asMap().keySet());
+						if (!keys.isEmpty()) {
+							Collections.sort(keys);
+							_scheduleInfoReadCount.set(keys.get(0));
+							continue;
+						} else {
+							break;
+						}
 					}
 				}
 				if (System.currentTimeMillis() > max)

zkdoc/release-note

@@ -40,6 +40,7 @@ ZK 10.0.0
   ZK-5151: Client MVVM - SelectedItem should be sync in Cascader (getter/setter)
   ZK-5152: Client MVVM - Cascader should use TreeSelectableModel
   ZK-4355: timebox' default cols is too small
+  ZK-5611: Executions.schedule() might fail if it's called by multiple threads
 
 * Upgrade Notes
   + Upgrade commons-fileupload to commons-fileupload2-javax 2.0.0-M2 and commons-io to 2.13.0 to support jakarta-friendly uploads

zktest/src/main/java/org/zkoss/zktest/test2/B96_ZK_5611_Composer.java

@@ -0,0 +1,54 @@
+/* B96_ZK_5611_Composer.java
+
+        Purpose:
+
+        Description:
+
+        History:
+                Thu Jan 25 17:11:30 CST 2024, Created by jamson
+
+Copyright (C) 2024 Potix Corporation. All Rights Reserved.
+*/
+package org.zkoss.zktest.test2;
+
+import java.util.concurrent.CompletableFuture;
+
+import org.zkoss.zk.ui.Component;
+import org.zkoss.zk.ui.Desktop;
+import org.zkoss.zk.ui.Executions;
+import org.zkoss.zk.ui.event.Event;
+import org.zkoss.zk.ui.select.SelectorComposer;
+import org.zkoss.zk.ui.select.annotation.Listen;
+import org.zkoss.zul.Hlayout;
+import org.zkoss.zul.Label;
+
+public class B96_ZK_5611_Composer extends SelectorComposer {
+
+	private int hlayoutId;
+	private Component comp;
+	private Desktop desktop;
+
+	@Override
+	public void doAfterCompose(Component comp) throws Exception {
+		super.doAfterCompose(comp);
+		this.comp = comp;
+		Executions.getCurrent().getDesktop().enableServerPush(true);
+		hlayoutId = 0;
+		desktop = comp.getDesktop();
+	}
+
+	@Listen("onClick = button")
+	public void start(){
+		Hlayout hlayout = new Hlayout();
+		hlayout.setId("hlayout" + hlayoutId++);
+		comp.appendChild(hlayout);
+		for (int i = 0 ; i < 3 ; i++) {
+			String taskId = String.valueOf(i);
+			CompletableFuture.runAsync(() -> {
+				Executions.schedule(desktop,  event -> {
+					hlayout.appendChild(new Label(taskId));
+				}, new Event("myEvent"));
+			});
+		}
+	}
+}

zktest/src/main/webapp/test2/B96-ZK-5611.zul

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+B96-ZK-5611.zul
+
+        Purpose:
+
+        Description:
+
+        History:
+                Thu Jan 25 17:10:14 CST 2024, Created by jamson
+
+Copyright (C) 2024 Potix Corporation. All Rights Reserved.
+-->
+<zk>
+	<label multiline="true">
+		1. Click start button, and you should see a new row with 3 threads' name below the button (e.g. 0 2 1).
+		2. Keep clicking the button, the order of threads might be different, but the number of the threads should ALWAYS be 3.
+		3. The name of 3 threads should be all-different.
+	</label>
+	<div apply="org.zkoss.zktest.test2.B96_ZK_5611_Composer">
+		<button label="schedule"/>
+	</div>
+</zk>

zktest/src/main/webapp/test2/config.properties

@@ -3072,6 +3072,7 @@ B90-ZK-4431.zul=A,E,Multislider
 ##manually##B96-ZK-5061.zul=A,E,embedded,CSS
 ##zats##B96-ZK-5379.zul=A,E,scroll,floatingScrollbar,anchornav
 ##zats##B96-ZK-5414.zul=A,E,timer,cascader
+##zats##B96-ZK-5611.zul=A,E,Executions,Schedule,MultiThread,Asynchronous,ServerPush
 
 ## B100
 ##zats##B100-ZK-5089.zul=A,E,Window,AfterSizeEvent

zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_5611Test.java

@@ -0,0 +1,48 @@
+/* B96_ZK_5611Test.java
+
+        Purpose:
+
+        Description:
+
+        History:
+                Thu Jan 25 18:07:48 CST 2024, Created by jamson
+
+Copyright (C) 2024 Potix Corporation. All Rights Reserved.
+*/
+package org.zkoss.zktest.zats.test2;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.HashSet;
+
+import org.junit.jupiter.api.Test;
+
+import org.zkoss.test.webdriver.WebDriverTestCase;
+import org.zkoss.test.webdriver.ztl.JQuery;
+
+public class B96_ZK_5611Test extends WebDriverTestCase {
+
+	@Test
+	public void test() {
+		connect();
+		for (int i = 0; i < 100; ++i)
+			click(jq("@button"));
+		try {
+			Thread.sleep(2000);
+		} catch (InterruptedException e) {
+			e.printStackTrace();
+		}
+
+		for (int i = 0; i < 100; ++i) {
+			JQuery children = jq("$hlayout" + i).eq(0).children();
+			// check whether the number of children is 3 (all threads exists)
+			assertEquals(3, children.length());
+
+			// check the 3 threads ID are 0, 1, 2 (no duplicate)
+			HashSet<String> ids = new HashSet<String>();
+			for (int j = 0; j < 3; ++j)
+				ids.add(children.get(j).firstChild().get("innerHTML"));
+			assertEquals(3, ids.size());
+		}
+	}
+}