Data passed out of thread by ocs internals should be deep copied
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 19
- Forks
- 7
- PR merge metrics
- No merged PRs in 30d
Description
Looking here --
https://github.com/simonsobs/ocs/blob/4199ac1474318692a06884e24395715d5838e2e6/ocs/ocs_feed.py#L204-L208
The little comment is right but the shallow .copy is not sufficient to protect that data.
The caller, often a data acquisition Process running in a thread (rather than reactor), is at risk of passing in the data structure and then modifying it, which can lead to quiet corruption (data from next frame appearing with wrong timestamps) or feed data processing errors (e.g. if data vectors with varying lengths are merged into data blocks, which will cause an exception just before publish to crossbar).
This issue led to patch of wiregrid encoder agent, here.
I think replacing copy with deepcopy is a sufficient solution here, since these structures are usually small simple dicts.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at ocs/ocs_feed.py lines 204-208 and inspect how data is passed out of the thread. Compare the related wiregrid encoder agent change in socs/pull/1043. Done means nested data is protected from caller mutation so later frames cannot corrupt timestamps or vector lengths before publication.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100