Applying a lock over a call to a method performing network transactions 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()
which sends a Page
object containing information being passed between a client and server. The method is synchronized to protect access to the array pageBuff
. Calling writeObject
within the synchronized sendPage
can lead to a deadlock for high latency or lossy network connections.
// 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){ try{ // 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 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 out.writeObject(targetPage); out.flush(); out.close(); } catch(IOException io){ /* handle exception */ } return SUCCESS; }
Compliant Solution
One compliant solution entails separating the actions into a sequence of steps:
- Perform actions on data structures requiring synchronization
- Create copies of Objects to send
- Perform network calls in a separate method
In the following example, the synchronized method getPage()
is called from SendReply
to find the appropriate Page
requested by the client from the Page
array pageBuff
. The method sendReply()
in turn calls the unsynchronized method sendPage()
to deliver the Page
.
public boolean sendReply(Socket socket, String pageName){ Page targetPage = getPage(pageName); if(targetPage == null) return FAILURE; return sendPage(socket, targetPage); } private synchronized Page getPage(String pageName){ 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 else return failure return FAILURE; } }
General recommendations for correct use of locks and synchronization appear in CON00-J. Use synchronization judiciously.
Risk Assessment
If monitor regions such as 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
[[Grosso 01]] Chapter 10: Serialization
[[JLS 05]] Chapter 17, Threads and Locks
[[Rotem 08]] Falacies of Distributed Computing Explained