Applying a lock over a call to Synchronizing 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); } catch(IOException io){ // If recovery is not possible return FAILURE return FAILURE; } finally { out.flush(); out.close(); } return SUCCESS; } |
Risk Assessment
If synchronized
methods and statements contain network transactional logic, temporary or permanent deadlocks may result.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
CON20- 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] |
...