HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Deadlock watchdog in a server to defend against poorly written extensions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
writtenextensionsdeadlockdefendpoorlyagainstserverwatchdog

Problem

In the Red5 server we have no control over what implementers do with their applications. As such, we have attempted to implement code that would prevent them from causing bad things to happen.

This class shows the creation of the task when a message comes in from a client:

ReceivedMessageTask task = new ReceivedMessageTask(sessionId, message, handler, this);
task.setMaxHandlingTimeout(maxHandlingTimeout);
ListenableFuture future = (ListenableFuture) executor.submitListenable(new ListenableFutureTask(task));


Full class

The following class is setup in such a way as to prevent a "handler" (which is the implementers application) from using all the cycles or crashing the server. To summarize, a deadlock guard with an allowed run time period interrupts an action that exceeds the time limit given.

I would like a review on the following code, which does seem to perform as intended on running servers.

```
public final class ReceivedMessageTask implements Callable {

private final static Logger log = LoggerFactory.getLogger(ReceivedMessageTask.class);

private final RTMPConnection conn;

private final IRTMPHandler handler;

private final String sessionId;

private Packet message;

private AtomicBoolean done = new AtomicBoolean(false);

private DeadlockGuard guard;

private long maxHandlingTime = 500L;

public ReceivedMessageTask(String sessionId, Packet message, IRTMPHandler handler) {
this(sessionId, message, handler, (RTMPConnection) RTMPConnManager.getInstance().getConnectionBySessionId(sessionId));
}

public ReceivedMessageTask(String sessionId, Packet message, IRTMPHandler handler, RTMPConnection conn) {
this.sessionId = sessionId;
this.message = message;
this.handler = handler;
this.conn = conn;
}

public Boolean call() throws Exception {
Red5.setConnectionLocal(conn);
try {
if (maxHandlingTime <= 0) {
if (!Red5.isDebug

Solution

Code looks fine - I have a few comments from practical point of view:

Instead of new Thread() to run something in separate thread - use thread pools with fixed number of threads allocated for it.

The problem you may have is if client threads which are calling API are not comtrolled with thread pool - you may run out of resources.
So you need to control submitListenable calls to be in the pool and have separate pool for your DeadlockGuard tasks with same size.

Also I would recommend wait/notify mechanism as this is more controllable from developer perspective instead of interrupts - but maybe your approach is also fine if it works.

Nit - private:

private AtomicBoolean sleeping = new AtomicBoolean(false);


Nit - good practice is to set timeout on join as well.

Code Snippets

private AtomicBoolean sleeping = new AtomicBoolean(false);

Context

StackExchange Code Review Q#74400, answer score: 2

Revisions (0)

No revisions yet.