Applying a lock over a call to a method performing network transactions can be problematic. Depending on the speed and reliability of the connection, the synchronization could lock up the program, producing temporary or permanent deadlock.
Noncompliant Code Example
This noncompliant code example involves the method send_page
which sends a Page
object containing information being passed between a client and server. send_packet
is synchronized to protect access to the array page_buff
.
public final boolean SUCCESS = true; public final boolean FAILURE = false; public synchronized boolean send_page(Socket socket, String page_name){ try{ // Get the output stream to write the Page to ObjectOutputStream outStream = new ObjectOutputStream(socket.getOutputStream()); Page target_page = null; // Find the Page requested by the client for(Page p : page_buff){ if(p.getName().equals(page_name)) target_page = p; } // Page requested does not exist if(target_page == null) return FAILURE; // Send the Page to the client out.writeObject(target_page); out.flush(); return SUCCESS; }catch(IOException io){ // handle exception } }
Calling writeObject
within the synchronized send_page
could lead to deadlock in the event that the connection becomes slow or acknowledgments of received data are delayed or lost.
Compliant Solution
A solution would be to separate actions into steps:
- Perform actions on data structures requiring synchronization.
- If sending data, create copies of Objects to send.
- Perform network calls in a separate method.
In the following example, the synchronized method get_page
is called to find the appropriate Page
requested by the client from the Page
array page_buff
. It then calls the method send_page
(which has no synchronization applied to it) to send the Page
public final boolean SUCCESS = true; public final boolean FAILURE = false; public boolean send_reply(Socket socket, String page_name){ Page target_page = get_page(page_name); if(target_page == null) return FAILURE; return send_page(Socket socket, target_page); } private synchronized Page get_page(String page_name){ Page target_page = null; for(Page p : page_buff){ if(p.getName().equals(page_name)) target_page = p; } return target_page; } public boolean send_page(Socket socket, Page page){ try{ // Get the output stream to write the Page to ObjectOutputStream outStream = new ObjectOutputStream(socket.getOutputStream()); // Send the Page to the client out.writeObject(page); out.flush(); return SUCCESS; }catch(IOException io){ // handle exception return FAILURE; } }
Risk Assessment
Application of synchronization or locks to methods that perform transactions over a network can lead to temporary or permanent deadlocks.
Rule |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
---|---|---|---|---|---|
CON37-J |
low |
unlikely |
high |
P1 |
L3 |