kura icon indicating copy to clipboard operation
kura copied to clipboard

Investigate PreparedRead thread safety

Open nicolatimeus opened this issue 6 years ago • 0 comments

The PreparedRead Javadoc reports that:

Performs the optimised read request described by this PreparedRead instance.
In order to improve efficiency this method can return the same ChannelRecord instances that were supplied as arguments to the Driver#prepareRead(List) call that created this PreparedRead.
The returned records should not be modified while a valid (not closed) PreparedRead holds a reference to them, otherwise unpredictable behaviour can occur.

The Javadoc above assumes that the PreparedRead is used by a single thread. This is not the case for the typical Driver client, that is the Asset. For example an Asset can be used by more than one thread in a Wire Graph, by the Web UI, by the Asset Cloudlet and possibly by other user applications.

Moreover, concurrency inside BaseAsset has been increased as part of the refactor process performed some time ago to avoid blocking Declarative Services threads due to blocking IO performed by the driver.

In most driver implementations the PreparedRead.getRecords() and PreparedRead.execute() returns always the same ChannelRecord instances, the same instances can be mutated by other threads concurrently, for example by executing the PreparedRead another time.

For example the following scenario can happen:

  1. An Asset executes a prepared read and starts converting the obtained ChannelRecords into WireRecords
  2. Another thread causes a read on the same asset, this will mutate the records that are being processed by 1.
  3. The WireRecords emitted by 1. end up containing data that comes from read 1. and 2.

In order to avoid this problem, a Driver should never mutate the records that have been returned by a PreparedRead, returning newly instantiated records every time getRecords() or execute() is called.

Allowing a PreparedRead to return the same record instances would require changes in Asset/Driver APIs and/or implementation.

nicolatimeus avatar Mar 20 '20 14:03 nicolatimeus