[QFJ-852] Missing synchronized blocks discovered in code review. Created: 01/Jul/15  Updated: 04/Jul/15

Status: Open
Project: QuickFIX/J
Component/s: Engine
Affects Version/s: 1.6.0
Fix Version/s: None

Type: Bug Priority: Default
Reporter: Nathan Tippy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: sequnece

Attachments: File quickfixjQFJ-852.patch    

 Description   

In the class Session in the following method

private boolean verify(Message msg, boolean checkTooHigh, boolean checkTooLow)

we find that synchronized (state.getLock()) is used to ensure atomic updates to the multiple fields in the state object. If this is needed then we must also check to ensure partial or dirty reads are avoided. This would happen any place that we call for more than one field of the state and assume the fields remain consistent with one another until we make a second or third request for other fields.

Directly after this same block the usage of isChunkedResendRequest, getCurrentEndSeqNo and getEndSeqNo run the risk of partial (dirty) reads because they are out side the sync.

This same problem also exists here where it may be the cause of other more serious issues.
private void nextSequenceReset(Message sequenceReset)

And to a a lesser degree in
private void doTargetTooHigh(Message msg)



 Comments   
Comment by Nathan Tippy [ 02/Jul/15 ]

For immediate use I apply this patch file to be safe. However this should be re-visited with a more elegant fix.

Comment by Christoph John [ 03/Jul/15 ]

Thanks for the patch!
Edit: did you encounter dirty read problems?

Comment by Nathan Tippy [ 04/Jul/15 ]

No I have not seen any problems but after discovery in code review I was uncomfortable running it without a fix.

I have not seen any negative consequences to applying the patch. So it was a defensive move.

Generated at Sat Nov 23 07:28:12 UTC 2024 using JIRA 7.5.2#75007-sha1:9f5725bb824792b3230a5d8716f0c13e296a3cae.