Skip to content

Add logic to address threading issues with writing from memory access#115

Merged
snideto merged 3 commits intojuergenH87:masterfrom
khauersp:feature/address-dm14-write-issue
Feb 4, 2026
Merged

Add logic to address threading issues with writing from memory access#115
snideto merged 3 commits intojuergenH87:masterfrom
khauersp:feature/address-dm14-write-issue

Conversation

@khauersp
Copy link
Contributor

We discovered a pretty nasty bug in the memory access class when it came to responding to write requests. We were never receiving the data to be written even though it was being seen on the bus. Turns out the blocking get in the dm14 sever was blocking the reception of the data. This is because the memory access _listen for dm14 was being called from the receive thread of the bus, but then the notifiy function (which was calling the respond method) was also being called from within the receive thread which then led to that thread getting blocked when we wait for the data. So the patch for that issue was to make a servicer thread that monitors when to call the notify function so that it will be called from a different thread and not block the receive thread.

@khauersp khauersp marked this pull request as draft January 29, 2026 18:20
mem_data = self.data_queue.get(block=True, timeout=max_timeout)
except queue.Empty:
self.reset_server()
raise RuntimeError("No data received from DM16 within timeout period")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to say DM16?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it say dm16 since that's the message for the data sending part of the transaction. Probably a slight abstraction leak though so I can remove it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah I just wasn't reading closely enough, looking again and it's clearly in the WAIT_FOR_DM16 case.

@khauersp khauersp marked this pull request as ready for review February 2, 2026 21:43
Copy link

@mattculler mattculler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@snideto snideto merged commit 01aba95 into juergenH87:master Feb 4, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants