Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions zk/src/org/zkoss/zk/ui/impl/DesktopImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -1683,12 +1683,14 @@ public void schedule(EventListener<T> listener, T event) {
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);
jumperchen marked this conversation as resolved.
Show resolved Hide resolved
}
_schedInfos.put(_scheduleInfoWriteCount.getAndIncrement(), new ScheduleInfo<T>(listener, event));
}
_schedInfos.put(_scheduleInfoWriteCount.getAndIncrement(), new ScheduleInfo<T>(listener, event));
}
});
}
Expand Down Expand Up @@ -1894,14 +1896,16 @@ public void onEvent(Event event) throws Exception {
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)
Expand Down
1 change: 1 addition & 0 deletions zkdoc/release-note
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ ZK 9.6.5

* Bugs
ZK-5453: XSS in chosenbox
ZK-5611: Executions.schedule() might fail if it's called by multiple threads

* Upgrade Notes

Expand Down
23 changes: 23 additions & 0 deletions zktest/src/archive/test2/B96-ZK-5611.zul
Original file line number Diff line number Diff line change
@@ -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>
1 change: 1 addition & 0 deletions zktest/src/archive/test2/config.properties
Original file line number Diff line number Diff line change
Expand Up @@ -3142,6 +3142,7 @@ B90-ZK-4431.zul=A,E,Multislider
##manually##B96-ZK-5427.zul=A,E,touch,context,mouse
##zats##B96-ZK-5414.zul=A,E,timer,cascader
##zats##B96-ZK-5453.zul=A,E,Chosenbox,XSS
##zats##B96-ZK-5611.zul=A,E,Executions,Schedule,MultiThread,Asynchronous,ServerPush

##
# Features - 3.0.x version
Expand Down
54 changes: 54 additions & 0 deletions zktest/src/org/zkoss/zktest/test2/B96_ZK_5611_Composer.java
Original file line number Diff line number Diff line change
@@ -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"));
});
}
}
}
48 changes: 48 additions & 0 deletions zktest/test/java/org/zkoss/zktest/zats/test2/B96_ZK_5611Test.java
Original file line number Diff line number Diff line change
@@ -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.Assert.assertEquals;

import java.util.HashSet;

import org.junit.Test;

import org.zkoss.zktest.zats.WebDriverTestCase;
import org.zkoss.zktest.zats.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());
}
}
}
Loading