Skip to content

Commit 47d62bf

Browse files
mondainmondain
committed
Fixed leaks in WebSocketScopes and JDKSchedulingService
1 parent 3b8e62c commit 47d62bf

File tree

5 files changed

+53
-23
lines changed

5 files changed

+53
-23
lines changed

‎common/src/main/java/org/red5/server/Server.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ public class Server implements IServer, ApplicationContextAware, InitializingBea
7272

7373
public Set<IConnectionListener> connectionListeners = new CopyOnWriteArraySet<IConnectionListener>();
7474

75+
// delay between posting a notification and informing any listeners
76+
public long notificationDelay = 5L;
77+
7578
/**
7679
* Setter for Spring application context
7780
*
@@ -297,7 +300,7 @@ public void removeListener(IConnectionListener listener) {
297300
* the scope that was created
298301
*/
299302
public void notifyScopeCreated(IScope scope) {
300-
schedulingService.addScheduledOnceJob(10, new ScheduledNotificationJob(JobAction.CREATED, scope));
303+
schedulingService.addScheduledOnceJob(notificationDelay, new ScheduledNotificationJob(JobAction.CREATED, scope));
301304
}
302305

303306
/**
@@ -307,7 +310,7 @@ public void notifyScopeCreated(IScope scope) {
307310
* the scope that was removed
308311
*/
309312
public void notifyScopeRemoved(IScope scope) {
310-
schedulingService.addScheduledOnceJob(10, new ScheduledNotificationJob(JobAction.REMOVED, scope));
313+
schedulingService.addScheduledOnceJob(notificationDelay, new ScheduledNotificationJob(JobAction.REMOVED, scope));
311314
}
312315

313316
/**
@@ -317,7 +320,7 @@ public void notifyScopeRemoved(IScope scope) {
317320
* the scope that was added
318321
*/
319322
public void notifyBasicScopeAdded(IBasicScope scope) {
320-
schedulingService.addScheduledOnceJob(10, new ScheduledNotificationJob(JobAction.BASIC_ADD, scope));
323+
schedulingService.addScheduledOnceJob(notificationDelay, new ScheduledNotificationJob(JobAction.BASIC_ADD, scope));
321324
}
322325

323326
/**
@@ -327,7 +330,7 @@ public void notifyBasicScopeAdded(IBasicScope scope) {
327330
* the scope that was removed
328331
*/
329332
public void notifyBasicScopeRemoved(IBasicScope scope) {
330-
schedulingService.addScheduledOnceJob(10, new ScheduledNotificationJob(JobAction.BASIC_REMOVE, scope));
333+
schedulingService.addScheduledOnceJob(notificationDelay, new ScheduledNotificationJob(JobAction.BASIC_REMOVE, scope));
331334
}
332335

333336
/**
@@ -337,7 +340,7 @@ public void notifyBasicScopeRemoved(IBasicScope scope) {
337340
* the new connection
338341
*/
339342
public void notifyConnected(IConnection conn) {
340-
schedulingService.addScheduledOnceJob(10, new ScheduledNotificationJob(JobAction.CONNECTED, conn));
343+
schedulingService.addScheduledOnceJob(notificationDelay, new ScheduledNotificationJob(JobAction.CONNECTED, conn));
341344
}
342345

343346
/**
@@ -347,7 +350,7 @@ public void notifyConnected(IConnection conn) {
347350
* the disconnected connection
348351
*/
349352
public void notifyDisconnected(final IConnection conn) {
350-
schedulingService.addScheduledOnceJob(10, new ScheduledNotificationJob(JobAction.DISCONNECTED, conn));
353+
schedulingService.addScheduledOnceJob(notificationDelay, new ScheduledNotificationJob(JobAction.DISCONNECTED, conn));
351354
}
352355

353356
// job actions for scope notifications

‎common/src/main/java/org/red5/server/adapter/MultiThreadedApplicationAdapter.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,7 @@ public String addScheduledJob(int interval, IScheduledJob job) {
972972
if (schedulingService == null) {
973973
schedulingService = (ISchedulingService) ScopeUtils.getScopeService(scope, ISchedulingService.class, JDKSchedulingService.class, false);
974974
}
975+
// the caller is requred to call removal one they have finished with this job
975976
return schedulingService.addScheduledJob(interval, job);
976977
}
977978

@@ -1025,6 +1026,7 @@ public String addScheduledJobAfterDelay(int interval, IScheduledJob job, int del
10251026
if (schedulingService == null) {
10261027
schedulingService = (ISchedulingService) ScopeUtils.getScopeService(scope, ISchedulingService.class, JDKSchedulingService.class, false);
10271028
}
1029+
// the caller is requred to call removal one they have finished with this job
10281030
return schedulingService.addScheduledJobAfterDelay(interval, job, delay);
10291031
}
10301032

‎common/src/main/java/org/red5/server/scheduling/JDKSchedulingService.java‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,8 @@ public String addScheduledJob(int interval, IScheduledJob job) {
8585
Map<String, Object> jobData = new HashMap<>();
8686
jobData.put(ISchedulingService.SCHEDULING_SERVICE, this);
8787
jobData.put(ISchedulingService.SCHEDULED_JOB, job);
88-
// runnable task
89-
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob();
90-
schedJob.setJobDataMap(jobData);
88+
// runnable task with false to prevent removal after it runs, since this job is meant to iterate
89+
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob(name, jobData, false);
9190
// schedule it to run at interval
9291
ScheduledFuture<?> future = scheduler.scheduleAtFixedRate(schedJob, interval, interval, TimeUnit.MILLISECONDS);
9392
// add to the key map
@@ -103,8 +102,7 @@ public String addScheduledOnceJob(Date date, IScheduledJob job) {
103102
jobData.put(ISchedulingService.SCHEDULING_SERVICE, this);
104103
jobData.put(ISchedulingService.SCHEDULED_JOB, job);
105104
// runnable task
106-
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob();
107-
schedJob.setJobDataMap(jobData);
105+
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob(name, jobData);
108106
// calculate the delay
109107
long delay = date.getTime() - System.currentTimeMillis();
110108
// schedule it to run once after the specified delay
@@ -123,8 +121,7 @@ public String addScheduledOnceJob(long timeDelta, IScheduledJob job) {
123121
jobData.put(ISchedulingService.SCHEDULING_SERVICE, this);
124122
jobData.put(ISchedulingService.SCHEDULED_JOB, job);
125123
// runnable task
126-
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob();
127-
schedJob.setJobDataMap(jobData);
124+
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob(name, jobData);
128125
// schedule it to run once after the specified delay
129126
ScheduledFuture<?> future = scheduler.schedule(schedJob, timeDelta, TimeUnit.MILLISECONDS);
130127
// add to the key map
@@ -139,9 +136,8 @@ public String addScheduledJobAfterDelay(int interval, IScheduledJob job, int del
139136
Map<String, Object> jobData = new HashMap<>();
140137
jobData.put(ISchedulingService.SCHEDULING_SERVICE, this);
141138
jobData.put(ISchedulingService.SCHEDULED_JOB, job);
142-
// runnable task
143-
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob();
144-
schedJob.setJobDataMap(jobData);
139+
// runnable task with false to prevent removal after it runs, since this job is meant to iterate
140+
JDKSchedulingServiceJob schedJob = new JDKSchedulingServiceJob(name, jobData, false);
145141
// schedule it to run at interval
146142
ScheduledFuture<?> future = scheduler.scheduleAtFixedRate(schedJob, delay, interval, TimeUnit.MILLISECONDS);
147143
// add to the key map
@@ -182,10 +178,11 @@ public void resumeScheduledJob(String name) {
182178
public void removeScheduledJob(String name) {
183179
try {
184180
ScheduledFuture<?> future = keyMap.remove(name);
185-
if (future != null) {
181+
// check done before we attempt cancelling
182+
if (future != null && !future.isDone()) {
186183
future.cancel(interruptOnRemove);
187184
} else {
188-
log.debug("No key found for job: {}", name);
185+
log.debug("No key found for job: {} or the job was done", name);
189186
}
190187
} catch (Exception ex) {
191188
throw new RuntimeException(ex);

‎common/src/main/java/org/red5/server/scheduling/JDKSchedulingServiceJob.java‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,37 @@
2222
*/
2323
public class JDKSchedulingServiceJob implements Runnable {
2424

25-
private Logger log = LoggerFactory.getLogger(JDKSchedulingServiceJob.class);
25+
private final static Logger log = LoggerFactory.getLogger(JDKSchedulingServiceJob.class);
2626

2727
/**
2828
* Job data map
2929
*/
30-
private Map<String, Object> jobDataMap;
30+
private final Map<String, Object> jobDataMap;
3131

32-
public void setJobDataMap(Map<String, Object> jobDataMap) {
32+
private final String jobName;
33+
34+
// set this flag to prevent removal within the internal run() of the scheduled job
35+
private final boolean autoRemove;
36+
37+
public JDKSchedulingServiceJob(String name, Map<String, Object> dataMap) {
38+
this.jobDataMap = dataMap;
39+
log.debug("Set job data map: {}", jobDataMap);
40+
this.jobName = name;
41+
this.autoRemove = true;
42+
}
43+
44+
public JDKSchedulingServiceJob(String name, Map<String, Object> dataMap, boolean autoRemove) {
45+
this.jobDataMap = dataMap;
3346
log.debug("Set job data map: {}", jobDataMap);
34-
this.jobDataMap = jobDataMap;
47+
this.jobName = name;
48+
this.autoRemove = autoRemove;
3549
}
3650

3751
public void run() {
3852
//log.debug("execute");
53+
ISchedulingService service = (ISchedulingService) jobDataMap.get(ISchedulingService.SCHEDULING_SERVICE);
3954
IScheduledJob job = null;
4055
try {
41-
ISchedulingService service = (ISchedulingService) jobDataMap.get(ISchedulingService.SCHEDULING_SERVICE);
4256
job = (IScheduledJob) jobDataMap.get(ISchedulingService.SCHEDULED_JOB);
4357
job.execute(service);
4458
} catch (Throwable e) {
@@ -47,6 +61,13 @@ public void run() {
4761
} else {
4862
log.warn("Job {} execution failed", job.toString(), e);
4963
}
64+
} finally {
65+
// remove the job
66+
if (autoRemove) {
67+
service.removeScheduledJob(jobName);
68+
}
69+
// clear the map
70+
jobDataMap.clear();
5071
}
5172
}
5273

‎server/src/main/java/org/red5/net/websocket/WebSocketScopeManager.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,14 @@ public boolean addWebSocketScope(WebSocketScope webSocketScope) {
165165
try {
166166
wsConn.sendPing(PING_BYTES);
167167
} catch (Exception e) {
168+
log.debug("Exception pinging connection: {} connection will be closed", wsConn.getSessionId(), e);
169+
// if the ping fails, consider them gone
170+
wsConn.close();
168171
}
172+
} else {
173+
log.debug("Removing unconnected connection: {} during ping loop", wsConn.getSessionId());
174+
// if the connection isn't connected, remove them
175+
wsScope.removeConnection(wsConn);
169176
}
170177
});
171178
log.trace("finished pinging scope: {}", sName);

0 commit comments

Comments
 (0)