Skip to content

Commit 7d7a068

Browse files
author
zhourenjian
committed
Fixing bug that managed pipe monitor may throw ConcurrentModificationException
The pipe status may be incorrect, trying to fix it
1 parent 7175187 commit 7d7a068

File tree

4 files changed

+137
-126
lines changed

4 files changed

+137
-126
lines changed

‎sources/net.sf.j2s.ajax/ajaxpipe/net/sf/j2s/ajax/CompoundPipeRunnable.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,12 @@ protected CompoundPipeSession getSession(String session) {
5050

5151
@Override
5252
public boolean pipeDestroy() {
53-
if (!super.pipeDestroy()) return false;
54-
5553
for (int i = 0; i < pipes.length; i++) {
5654
if (pipes[i] != null) {
5755
pipes[i].pipeDestroy();
5856
}
5957
}
60-
return true;
58+
return super.pipeDestroy();
6159
}
6260

6361
@Override

‎sources/net.sf.j2s.ajax/ajaxpipe/net/sf/j2s/ajax/ManagedPipeHelper.java‎

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,27 @@
11
package net.sf.j2s.ajax;
22

3-
import java.util.Collections;
43
import java.util.HashMap;
54
import java.util.HashSet;
65
import java.util.Iterator;
76
import java.util.Map;
87
import java.util.Set;
8+
import java.util.concurrent.locks.ReentrantLock;
99

1010
import net.sf.j2s.annotation.J2SIgnore;
1111

1212
public class ManagedPipeHelper {
1313

1414
@J2SIgnore
1515
private static Map<String, SimplePipeRunnable> pipeSessions = null;
16+
1617
@J2SIgnore
1718
private static boolean monitored = false;
1819

1920
@J2SIgnore
20-
private static boolean monitorRunning = false;
21+
private static long monitoringInterval = 10000; // 10s
2122

2223
@J2SIgnore
23-
private static long monitoringInterval = 10000; // 10s
24-
24+
private static ReentrantLock lock = new ReentrantLock();
2525

2626
@J2SIgnore
2727
public static long getMonitoringInterval() {
@@ -34,24 +34,11 @@ public static void setMonitoringInterval(long monitoringInterval) {
3434
}
3535

3636
@J2SIgnore
37-
public static void stopMonitor() {
38-
monitorRunning = false;
39-
}
40-
41-
static synchronized void init() {
42-
if (pipeSessions == null) {
43-
pipeSessions = Collections.synchronizedMap(new HashMap<String, SimplePipeRunnable>(20));
44-
}
45-
}
46-
47-
@J2SIgnore
48-
static void startMonitor() {
49-
init();
50-
monitorRunning = true;
51-
new Thread(new Runnable() {
37+
private static void startMonitor() {
38+
Thread thread = new Thread("Managed Pipe Session Monitor") {
5239

5340
public void run() {
54-
while (monitorRunning) {
41+
while (true) {
5542
try {
5643
Thread.sleep(getMonitoringInterval());
5744
} catch (InterruptedException e) {
@@ -60,7 +47,15 @@ public void run() {
6047
System.err.println("Pipe sessions are null or empty! Managed pipe session monitor exited!");
6148
break;
6249
}
63-
Set<SimplePipeRunnable> pipes = new HashSet<SimplePipeRunnable>(pipeSessions.values());
50+
Set<SimplePipeRunnable> pipes = null;
51+
try {
52+
synchronized (pipeSessions) {
53+
pipes = new HashSet<SimplePipeRunnable>(pipeSessions.values());
54+
}
55+
} catch (Throwable e) {
56+
e.printStackTrace();
57+
continue; //
58+
}
6459
for (Iterator<SimplePipeRunnable> itr = pipes.iterator(); itr.hasNext();) {
6560
SimplePipeRunnable pipe = itr.next();
6661
try {
@@ -80,7 +75,9 @@ public void run() {
8075
} catch (Throwable e) {
8176
e.printStackTrace();
8277
}
83-
pipeSessions.remove(pipe.pipeKey);
78+
synchronized (pipeSessions) {
79+
pipeSessions.remove(pipe.pipeKey);
80+
}
8481
}
8582
} else {
8683
pipe.lastLiveDetected = System.currentTimeMillis();
@@ -89,22 +86,41 @@ public void run() {
8986
e.printStackTrace();
9087
}
9188
}
89+
lock.lock();
90+
try {
91+
if (pipeSessions == null || pipeSessions.isEmpty()) {
92+
monitored = false;
93+
break;
94+
}
95+
} finally {
96+
lock.unlock();
97+
}
9298
}
9399
}
94100

95-
}, "Managed Pipe Session Monitor").start();
101+
};
102+
thread.setDaemon(true);
103+
thread.start();
96104
}
97105

98106
@J2SIgnore
99107
public static void monitoringPipe(SimplePipeRunnable pipe) {
100-
init();
101-
pipeSessions.put(pipe.pipeKey, pipe);
102-
pipe.lastLiveDetected = System.currentTimeMillis();
103-
if (monitored) {
104-
return;
105-
}
106-
monitored = true;
107-
startMonitor();
108+
long now = System.currentTimeMillis();
109+
lock.lock();
110+
try {
111+
if (pipeSessions == null) {
112+
pipeSessions = new HashMap<String, SimplePipeRunnable>(20);
113+
}
114+
pipeSessions.put(pipe.pipeKey, pipe);
115+
pipe.lastLiveDetected = now;
116+
if (monitored) {
117+
return;
118+
}
119+
monitored = true;
120+
} finally {
121+
lock.unlock();
122+
}
123+
startMonitor();
108124
}
109125

110126
}

‎sources/net.sf.j2s.ajax/ajaxpipe/net/sf/j2s/ajax/SimplePipeHelper.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,11 @@ static String nextPipeKey() {
114114
static void removePipe(String key) {
115115
if (pipes == null || key == null) return;
116116
SimplePipeRunnable removedPipe = pipes.remove(key);
117-
if (removedPipe != null) {
118-
if (pipeMap != null) {
119-
Vector<SimpleSerializable> removedVector = pipeMap.remove(key);
120-
if (removedVector != null) {
121-
synchronized (removedVector) {
122-
removedVector.notifyAll();
123-
}
117+
if (removedPipe != null && pipeMap != null) {
118+
Vector<SimpleSerializable> removedVector = pipeMap.remove(key);
119+
if (removedVector != null) {
120+
synchronized (removedVector) {
121+
removedVector.notifyAll();
124122
}
125123
}
126124
}
@@ -191,7 +189,7 @@ static boolean isPipeLive(String key) {
191189
@J2SIgnore
192190
static boolean notifyPipeStatus(String key, boolean live) {
193191
SimplePipeRunnable pipe = getPipe(key);
194-
if (pipe != null) {
192+
if (pipe != null && pipe.isPipeLive()) {
195193
pipe.updateStatus(live);
196194
return true;
197195
}

‎sources/net.sf.j2s.ajax/ajaxpipe/net/sf/j2s/ajax/SimplePipeHttpServlet.java‎

Lines changed: 83 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -182,101 +182,100 @@ protected void doPipe(final HttpServletResponse resp, String key, String type, S
182182
writer = resp.getWriter();
183183
}
184184

185-
SimplePipeHelper.notifyPipeStatus(key, true); // update it!
186-
187185
long lastPipeDataWritten = -1;
188-
189186
long beforeLoop = new Date().getTime();
190-
Vector<SimpleSerializable> vector = null;
191-
int priority = 0;
192-
long lastLiveDetected = System.currentTimeMillis();
193-
SimplePipeRunnable pipe = SimplePipeHelper.getPipe(key);
194-
long waitClosingInterval = 5000;
195-
if (pipe != null) {
196-
waitClosingInterval = pipe.pipeWaitClosingInterval();
197-
}
198-
while ((vector = SimplePipeHelper.getPipeVector(key)) != null
199-
/* && SimplePipeHelper.isPipeLive(key) */ // check it!
200-
&& !writer.checkError()) {
201-
if (!SimplePipeHelper.isPipeLive(key)) {
202-
if (System.currentTimeMillis() - lastLiveDetected > waitClosingInterval) {
203-
// break out while loop so pipe connection will be closed
204-
break;
205-
} else { // sleep 1s and continue to check pipe status again
206-
try {
207-
Thread.sleep(1000);
208-
} catch (InterruptedException e) {
187+
if (SimplePipeHelper.notifyPipeStatus(key, true)) { // update it!
188+
Vector<SimpleSerializable> vector = null;
189+
int priority = 0;
190+
long lastLiveDetected = System.currentTimeMillis();
191+
SimplePipeRunnable pipe = SimplePipeHelper.getPipe(key);
192+
long waitClosingInterval = 5000;
193+
if (pipe != null) {
194+
waitClosingInterval = pipe.pipeWaitClosingInterval();
195+
}
196+
while ((vector = SimplePipeHelper.getPipeVector(key)) != null
197+
/* && SimplePipeHelper.isPipeLive(key) */ // check it!
198+
&& !writer.checkError()) {
199+
if (!SimplePipeHelper.isPipeLive(key)) {
200+
if (System.currentTimeMillis() - lastLiveDetected > waitClosingInterval) {
201+
// break out while loop so pipe connection will be closed
202+
break;
203+
} else { // sleep 1s and continue to check pipe status again
204+
try {
205+
Thread.sleep(1000);
206+
} catch (InterruptedException e) {
207+
}
208+
continue;
209209
}
210-
continue;
210+
} else {
211+
lastLiveDetected = System.currentTimeMillis();
211212
}
212-
} else {
213-
lastLiveDetected = System.currentTimeMillis();
214-
}
215-
int size = vector.size();
216-
if (size > 0) {
217-
for (int i = 0; i < size; i++) {
218-
SimpleSerializable ss = null;
219-
/*
220-
* Still need to check vector size!
221-
* Maybe multiple pipe servlets!
222-
*/
223-
synchronized (vector) {
224-
if (vector.size() <= 0) break;
225-
ss = vector.remove(0);
226-
}
227-
if (ss == null) break; // terminating signal
228-
output(writer, type, key, ss.serialize());
229-
lastPipeDataWritten = new Date().getTime();
230-
writer.flush();
231-
if (ss instanceof ISimplePipePriority) {
232-
ISimplePipePriority spp = (ISimplePipePriority) ss;
233-
int p = spp.getPriority();
234-
if (p <= 0) {
235-
p = ISimplePipePriority.IMPORTANT;
213+
int size = vector.size();
214+
if (size > 0) {
215+
for (int i = 0; i < size; i++) {
216+
SimpleSerializable ss = null;
217+
/*
218+
* Still need to check vector size!
219+
* Maybe multiple pipe servlets!
220+
*/
221+
synchronized (vector) {
222+
if (vector.size() <= 0) break;
223+
ss = vector.remove(0);
224+
}
225+
if (ss == null) break; // terminating signal
226+
output(writer, type, key, ss.serialize());
227+
lastPipeDataWritten = new Date().getTime();
228+
writer.flush();
229+
if (ss instanceof ISimplePipePriority) {
230+
ISimplePipePriority spp = (ISimplePipePriority) ss;
231+
int p = spp.getPriority();
232+
if (p <= 0) {
233+
p = ISimplePipePriority.IMPORTANT;
234+
}
235+
priority += p;
236+
} else {
237+
priority += ISimplePipePriority.IMPORTANT;
236238
}
237-
priority += p;
238-
} else {
239-
priority += ISimplePipePriority.IMPORTANT;
240239
}
240+
} else {
241+
writer.flush();
241242
}
242-
} else {
243-
writer.flush();
244-
}
245-
/*
246-
* Client should send in "notify" request to simulate the following #notifyPipeStatus
247-
*/
248-
// SimplePipeHelper.notifyPipeStatus(key, true);
249-
250-
long now = new Date().getTime();
251-
if ((lastPipeDataWritten == -1 && now - beforeLoop >= pipeQueryTimeout)
252-
|| (lastPipeDataWritten > 0
253-
&& now - lastPipeDataWritten >= pipeQueryTimeout
254-
&& SimplePipeRequest.PIPE_TYPE_CONTINUUM.equals(type))) {
255-
output(writer, type, key, SimplePipeRequest.PIPE_STATUS_OK);
256-
lastPipeDataWritten = new Date().getTime();
257-
}
258-
259-
now = new Date().getTime();
260-
if ((vector = SimplePipeHelper.getPipeVector(key)) != null // may be broken down already!!
261-
&& (SimplePipeRequest.PIPE_TYPE_CONTINUUM.equals(type)
262-
|| (SimplePipeRequest.PIPE_TYPE_SCRIPT.equals(type)
263-
&& now - beforeLoop < pipeScriptBreakout)
264-
|| (priority < ISimplePipePriority.IMPORTANT
265-
&& now - beforeLoop < pipeQueryTimeout))) {
266-
synchronized (vector) {
267-
try {
268-
vector.wait(1000);
269-
} catch (InterruptedException e) {
270-
e.printStackTrace();
243+
/*
244+
* Client should send in "notify" request to simulate the following #notifyPipeStatus
245+
*/
246+
// SimplePipeHelper.notifyPipeStatus(key, true);
247+
248+
long now = new Date().getTime();
249+
if ((lastPipeDataWritten == -1 && now - beforeLoop >= pipeQueryTimeout)
250+
|| (lastPipeDataWritten > 0
251+
&& now - lastPipeDataWritten >= pipeQueryTimeout
252+
&& SimplePipeRequest.PIPE_TYPE_CONTINUUM.equals(type))) {
253+
output(writer, type, key, SimplePipeRequest.PIPE_STATUS_OK);
254+
lastPipeDataWritten = new Date().getTime();
255+
}
256+
257+
now = new Date().getTime();
258+
if ((vector = SimplePipeHelper.getPipeVector(key)) != null // may be broken down already!!
259+
&& (SimplePipeRequest.PIPE_TYPE_CONTINUUM.equals(type)
260+
|| (SimplePipeRequest.PIPE_TYPE_SCRIPT.equals(type)
261+
&& now - beforeLoop < pipeScriptBreakout)
262+
|| (priority < ISimplePipePriority.IMPORTANT
263+
&& now - beforeLoop < pipeQueryTimeout))) {
264+
synchronized (vector) {
265+
try {
266+
vector.wait(1000);
267+
} catch (InterruptedException e) {
268+
e.printStackTrace();
269+
}
271270
}
271+
} else {
272+
break;
272273
}
273-
} else {
274-
break;
275-
}
276-
} // end of while
274+
} // end of while
275+
} // else pips is already closed or in other statuses
277276
if (SimplePipeHelper.getPipeVector(key) == null
278277
|| !SimplePipeHelper.isPipeLive(key)) { // pipe is tore down!
279-
SimplePipeHelper.notifyPipeStatus(key, false);
278+
//SimplePipeHelper.notifyPipeStatus(key, false); // Leave for pipe monitor to destroy it
280279
SimplePipeHelper.pipeTearDown(key);
281280
SimplePipeHelper.removePipe(key);
282281
try {

0 commit comments

Comments
 (0)