open-dis / opendis7-java

Project to provide a complete type-safe Java implementation of the DIS Protocol version 7 (IEEE 1278.1-2012) and SISO-REF-010 Enumerations specifications.
Other
6 stars 5 forks source link

Thread safety for PDU classes #24

Closed brutzman closed 8 months ago

brutzman commented 9 months ago

Found an interesting problem in DisThreadedNetworkInterface.selfTest() method where ESPDU changes to marking values were getting clobbered during fast sends/updates. This was shown by print statements showing the same marking value in rapid progression (rather than marking 1, marking 2, etc.).

Avoided this problem (for now) by creating a new PDU each time. I think this anomaly indicates that the library isn't fully thread safe, e.g. an ESPDU instance might get modified while still being sent (or reported upon). Some more thought will be needed for making all PDU methods thread safe.

Log of test results found in DisThreadedNetworkInterfaceSelfTestLog.txt

brutzman commented 9 months ago

Problem definition:

Any performance reduction from synchronization is miniscule in comparison to possible loss of object integrity. Indeed synchronization blocking only occurs if it is needed. If we inadvertently create a deadlock, that will certainly be discovered as part of our extensive version-controlled unit tests and example applications - further fine-tuning can then occur.

Next steps:

  1. Done. For DisThreadedNetworkInterface.sendPDU() method, make a deep copy of the PDU prior to adding it to the buffered list for threaded sending. Java garbage collection should dispose of these extra objects reasonably well.
  1. Done. For JavaGenerator creation of each of the concrete PDU classes, add keyword synchronized to
    • all set accessor methods
    • all clear, copy and equal* accessor methods
    • all marshall and unmarshall accessor methods
    • all toString accessor methods
terry-norbraten commented 9 months ago

FixedAndVariableDatumRoundTripTest is now fixed (Issue #26)

brutzman commented 8 months ago

@terry-norbraten has also fixed some unnecessary duplicate creation of DISThreadedNetworkInterface instance in PduRecorder, further helping with threading.

We are looking pretty good, and so closing.