Applying a lock over a call to a method performing network transactions or declaring such a method synchronized can be problematic. Depending on the speed and reliability of the connection, synchronization can stall the program indefinitely causing a huge performance hit. At other times, it can result in temporary or permanent deadlock.
Noncompliant Code Example
This noncompliant code example involves the method sendPage() that sends a Page object containing information being passed between a client and a server. The method is synchronized to protect access to the array pageBuff. Calling writeObject() within the synchronized sendPage can result in a deadlock condition in high latency networks or when network connections are inherently lossy.
| Code Block | ||
|---|---|---|
| ||
// Class Page is defined separately. It stores and returns the Page name via getName()
public final boolean SUCCESS = true;
public final boolean FAILURE = false;
Page[] pageBuff = new Page[MAX_PAGE_SIZE];
public synchronized boolean sendPage(Socket socket, String pageName) throws IOException {
// Get the output stream to write the Page to
ObjectOutputStream out = new ObjectOutputStream(socket.getOutputStream());
Page targetPage = null;
// Find the Page requested by the client (this operation requires synchronization)
for(Page p : pageBuff) {
if(p.getName().compareTo(pageName) == 0) {
targetPage = p;
}
}
// Page requested does not exist
if(targetPage == null) {
return FAILURE;
}
// Send the Page to the client (does not require any synchronization)
out.writeObject(targetPage);
out.flush();
out.close();
return SUCCESS;
}
|
Compliant Solution
This compliant solution entails separating the actions into a sequence of steps:
...
| Code Block | ||
|---|---|---|
| ||
public boolean sendReply(Socket socket, String pageName) { // No synchronization
Page targetPage = getPage(pageName);
if(targetPage == null)
return FAILURE;
return sendPage(socket, targetPage);
}
private synchronized Page getPage(String pageName) { // Requires synchronization
Page targetPage = null;
for(Page p : pageBuff) {
if(p.getName().equals(pageName)) {
targetPage = p;
}
}
return targetPage;
}
public boolean sendPage(Socket socket, Page page){
try{
// Get the output stream to write the Page to
ObjectOutputStream out = new ObjectOutputStream(socket.getOutputStream());
// Send the Page to the client
out.writeObject(page);
out.flush();
out.close();
return SUCCESS;
}catch(IOException io){
// Handle exception
}
return FAILURE;
}
|
Risk Assessment
If synchronized methods and statements contain network transactional logic, temporary or permanent deadlocks may result.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
CON37- J | low | probable | high | P2 | L3 |
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
| Wiki Markup |
|---|
\[[Grosso 01|AA. Java References#Grosso 01]\] [Chapter 10: Serialization|http://oreilly.com/catalog/javarmi/chapter/ch10.html] \[[JLS 05|AA. Java References#JLS 05]\] [Chapter 17, Threads and Locks|http://java.sun.com/docs/books/jls/third_edition/html/memory.html] \[[Rotem 08|AA. Java References#Rotem 08]\] [Falacies of Distributed Computing Explained|http://www.rgoarchitects.com/Files/fallacies.pdf] |
...