mirror of
https://bitbucket.org/librepilot/librepilot.git
synced 2024-12-02 10:24:11 +01:00
AndroidGCS Telemetry: Use more finely grained semaphores to fix a deadlock
between UAVTalk and Telemetry. 1. processInputStream -> updateObjReq (locks uavtalk) -> tranactionCompleted (locks transInfo) 2. transactionTimeout (locks transInfo) -> sendObjectRequest -> ? -> setupTransaction (locks uavtalk)
This commit is contained in:
James Cotton
parent
c142deaa9d
commit
7eac1e2245
@ -202,7 +202,7 @@ public class Telemetry {
|
||||
/**
|
||||
* Register a new object for periodic updates (if enabled)
|
||||
*/
|
||||
private synchronized void registerObject(UAVObject obj) {
|
||||
private void registerObject(UAVObject obj) {
|
||||
// Setup object for periodic updates
|
||||
addObject(obj);
|
||||
|
||||
@ -213,24 +213,27 @@ public class Telemetry {
|
||||
/**
|
||||
* Add an object in the list used for periodic updates
|
||||
*/
|
||||
private synchronized void addObject(UAVObject obj) {
|
||||
// Check if object type is already in the list
|
||||
ListIterator<ObjectTimeInfo> li = objList.listIterator();
|
||||
while (li.hasNext()) {
|
||||
ObjectTimeInfo n = li.next();
|
||||
if (n.obj.getObjID() == obj.getObjID()) {
|
||||
// Object type (not instance!) is already in the list, do
|
||||
// nothing
|
||||
return;
|
||||
}
|
||||
}
|
||||
private void addObject(UAVObject obj) {
|
||||
|
||||
// If this point is reached, then the object type is new, let's add it
|
||||
ObjectTimeInfo timeInfo = new ObjectTimeInfo();
|
||||
timeInfo.obj = obj;
|
||||
timeInfo.timeToNextUpdateMs = 0;
|
||||
timeInfo.updatePeriodMs = 0;
|
||||
objList.add(timeInfo);
|
||||
synchronized (objList) {
|
||||
// Check if object type is already in the list
|
||||
ListIterator<ObjectTimeInfo> li = objList.listIterator();
|
||||
while (li.hasNext()) {
|
||||
ObjectTimeInfo n = li.next();
|
||||
if (n.obj.getObjID() == obj.getObjID()) {
|
||||
// Object type (not instance!) is already in the list, do
|
||||
// nothing
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// If this point is reached, then the object type is new, let's add it
|
||||
ObjectTimeInfo timeInfo = new ObjectTimeInfo();
|
||||
timeInfo.obj = obj;
|
||||
timeInfo.timeToNextUpdateMs = 0;
|
||||
timeInfo.updatePeriodMs = 0;
|
||||
objList.add(timeInfo);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -281,7 +284,7 @@ public class Telemetry {
|
||||
* Connect to all instances of an object depending on the event mask
|
||||
* specified
|
||||
*/
|
||||
private synchronized void connectToObjectInstances(UAVObject obj,
|
||||
private void connectToObjectInstances(UAVObject obj,
|
||||
int eventMask) {
|
||||
List<UAVObject> objs = objMngr.getObjectInstances(obj.getObjID());
|
||||
ListIterator<UAVObject> li = objs.listIterator();
|
||||
@ -312,43 +315,46 @@ public class Telemetry {
|
||||
* Update an object based on its metadata properties
|
||||
*/
|
||||
private void updateObject(UAVObject obj) {
|
||||
// Get metadata
|
||||
UAVObject.Metadata metadata = obj.getMetadata();
|
||||
|
||||
// Setup object depending on update mode
|
||||
int eventMask;
|
||||
if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_PERIODIC) {
|
||||
// Set update period
|
||||
setUpdatePeriod(obj, metadata.gcsTelemetryUpdatePeriod);
|
||||
// Connect signals for all instances
|
||||
eventMask = EV_UPDATED_MANUAL | EV_UPDATE_REQ;
|
||||
if (obj.isMetadata())
|
||||
eventMask |= EV_UNPACKED; // we also need to act on remote
|
||||
// updates (unpack events)
|
||||
synchronized(obj) {
|
||||
// Get metadata
|
||||
UAVObject.Metadata metadata = obj.getMetadata();
|
||||
|
||||
connectToObjectInstances(obj, eventMask);
|
||||
} else if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_ONCHANGE) {
|
||||
// Set update period
|
||||
setUpdatePeriod(obj, 0);
|
||||
// Connect signals for all instances
|
||||
eventMask = EV_UPDATED | EV_UPDATED_MANUAL | EV_UPDATE_REQ;
|
||||
if (obj.isMetadata())
|
||||
eventMask |= EV_UNPACKED; // we also need to act on remote
|
||||
// updates (unpack events)
|
||||
// Setup object depending on update mode
|
||||
int eventMask;
|
||||
if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_PERIODIC) {
|
||||
// Set update period
|
||||
setUpdatePeriod(obj, metadata.gcsTelemetryUpdatePeriod);
|
||||
// Connect signals for all instances
|
||||
eventMask = EV_UPDATED_MANUAL | EV_UPDATE_REQ;
|
||||
if (obj.isMetadata())
|
||||
eventMask |= EV_UNPACKED; // we also need to act on remote
|
||||
// updates (unpack events)
|
||||
|
||||
connectToObjectInstances(obj, eventMask);
|
||||
} else if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_THROTTLED) {
|
||||
// TODO
|
||||
} else if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_MANUAL) {
|
||||
// Set update period
|
||||
setUpdatePeriod(obj, 0);
|
||||
// Connect signals for all instances
|
||||
eventMask = EV_UPDATED_MANUAL | EV_UPDATE_REQ;
|
||||
if (obj.isMetadata())
|
||||
eventMask |= EV_UNPACKED; // we also need to act on remote
|
||||
// updates (unpack events)
|
||||
connectToObjectInstances(obj, eventMask);
|
||||
} else if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_ONCHANGE) {
|
||||
// Set update period
|
||||
setUpdatePeriod(obj, 0);
|
||||
// Connect signals for all instances
|
||||
eventMask = EV_UPDATED | EV_UPDATED_MANUAL | EV_UPDATE_REQ;
|
||||
if (obj.isMetadata())
|
||||
eventMask |= EV_UNPACKED; // we also need to act on remote
|
||||
// updates (unpack events)
|
||||
|
||||
connectToObjectInstances(obj, eventMask);
|
||||
connectToObjectInstances(obj, eventMask);
|
||||
} else if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_THROTTLED) {
|
||||
// TODO
|
||||
} else if (metadata.GetGcsTelemetryUpdateMode() == UAVObject.UpdateMode.UPDATEMODE_MANUAL) {
|
||||
// Set update period
|
||||
setUpdatePeriod(obj, 0);
|
||||
// Connect signals for all instances
|
||||
eventMask = EV_UPDATED_MANUAL | EV_UPDATE_REQ;
|
||||
if (obj.isMetadata())
|
||||
eventMask |= EV_UNPACKED; // we also need to act on remote
|
||||
// updates (unpack events)
|
||||
|
||||
connectToObjectInstances(obj, eventMask);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -448,7 +454,7 @@ public class Telemetry {
|
||||
registerObject(obj);
|
||||
}
|
||||
|
||||
private synchronized void newInstance(UAVObject obj) {
|
||||
private void newInstance(UAVObject obj) {
|
||||
registerObject(obj);
|
||||
}
|
||||
|
||||
|
||||
@ -269,16 +269,18 @@ public class UAVTalk {
|
||||
* it wants to give up on one (after a timeout) then it can cancel it
|
||||
* @return True if that object was pending, False otherwise
|
||||
*/
|
||||
public synchronized boolean cancelPendingTransaction(UAVObject obj) {
|
||||
if(respObj != null && respObj.getObjID() == obj.getObjID()) {
|
||||
if(transactionListener != null) {
|
||||
Log.d(TAG,"Canceling transaction: " + respObj.getName());
|
||||
transactionListener.TransactionFailed(respObj);
|
||||
}
|
||||
respObj = null;
|
||||
return true;
|
||||
} else
|
||||
return false;
|
||||
public boolean cancelPendingTransaction(UAVObject obj) {
|
||||
synchronized (respObj) {
|
||||
if(respObj != null && respObj.getObjID() == obj.getObjID()) {
|
||||
if(transactionListener != null) {
|
||||
Log.d(TAG,"Canceling transaction: " + respObj.getName());
|
||||
transactionListener.TransactionFailed(respObj);
|
||||
}
|
||||
respObj = null;
|
||||
return true;
|
||||
} else
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -296,15 +298,16 @@ public class UAVTalk {
|
||||
/**
|
||||
* This is the code that sets up a new UAVTalk packet that expects a response.
|
||||
*/
|
||||
private synchronized void setupTransaction(UAVObject obj, boolean allInstances, int type) {
|
||||
private void setupTransaction(UAVObject obj, boolean allInstances, int type) {
|
||||
synchronized (this) {
|
||||
// Only cancel if it is for a different object
|
||||
if(respObj != null && respObj.getObjID() != obj.getObjID())
|
||||
cancelPendingTransaction(obj);
|
||||
|
||||
// Only cancel if it is for a different object
|
||||
if(respObj != null && respObj.getObjID() != obj.getObjID())
|
||||
cancelPendingTransaction(obj);
|
||||
|
||||
respObj = obj;
|
||||
respAllInstances = allInstances;
|
||||
respType = type;
|
||||
respObj = obj;
|
||||
respAllInstances = allInstances;
|
||||
respType = type;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -315,7 +318,7 @@ public class UAVTalk {
|
||||
* Success (true), Failure (false)
|
||||
* @throws IOException
|
||||
*/
|
||||
private synchronized boolean objectTransaction(UAVObject obj, int type, boolean allInstances) throws IOException {
|
||||
private boolean objectTransaction(UAVObject obj, int type, boolean allInstances) throws IOException {
|
||||
if (type == TYPE_OBJ_ACK || type == TYPE_OBJ_REQ || type == TYPE_OBJ) {
|
||||
return transmitObject(obj, type, allInstances);
|
||||
} else {
|
||||
@ -701,21 +704,30 @@ public class UAVTalk {
|
||||
* Called when an object is received to check if this completes
|
||||
* a UAVTalk transaction
|
||||
*/
|
||||
private synchronized void updateObjReq(UAVObject obj) {
|
||||
private void updateObjReq(UAVObject obj) {
|
||||
// Check if this is not a possible candidate
|
||||
Assert.assertNotNull(obj);
|
||||
|
||||
if(respObj != null && respType == TYPE_OBJ_REQ && respObj.getObjID() == obj.getObjID() &&
|
||||
((respObj.getInstID() == obj.getInstID() || !respAllInstances))) {
|
||||
boolean succeeded = false;
|
||||
|
||||
// Indicate complete
|
||||
respObj = null;
|
||||
// The lock on UAVTalk must be release before the transaction succeeded signal is sent
|
||||
// because otherwise if a transaction timeout occurs at the same time we can get a
|
||||
// deadlock:
|
||||
// 1. processInputStream -> updateObjReq (locks uavtalk) -> tranactionCompleted (locks transInfo)
|
||||
// 2. transactionTimeout (locks transInfo) -> sendObjectRequest -> É -> setupTransaction (locks uavtalk)
|
||||
synchronized(this) {
|
||||
if(respObj != null && respType == TYPE_OBJ_REQ && respObj.getObjID() == obj.getObjID() &&
|
||||
((respObj.getInstID() == obj.getInstID() || !respAllInstances))) {
|
||||
|
||||
// Notify listener
|
||||
if (transactionListener != null)
|
||||
transactionListener.TransactionSucceeded(obj);
|
||||
// Indicate complete
|
||||
respObj = null;
|
||||
succeeded = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Notify listener
|
||||
if (succeeded && transactionListener != null)
|
||||
transactionListener.TransactionSucceeded(obj);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Loading…
Reference in New Issue
Block a user