Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to be thread-safe #12

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Conversation

Tom-Willemsen
Copy link

ISISComputingGroup/IBEX#7470

The error in the ticket looks to be caused by non thread-safe access.

Thread 1 Thread 2
var count = getChannelCount() -
for(int i=0; i<count; ++i) { -
- channels = <new smaller list>
getChannel(i) (boom) -

I've refactored it to use thread-safe iterators over CopyOnWriteArrayList in one case, and iterators over an explicit copy of an ArrayList in another similar case. Both of these approaches should be thread-safe against the scenario described above, which looks to be the only possible cause of the stack trace seen in the ticket.


Unfortunately I wasn't able to find a way to reliably reproduce the original issue in order to write tests for it. My attempt to reproduce was:

  • Set up config 1 with 250 blocks
  • Set up config 2 with 150 blocks (to maximise amount of time spent in the for loop above)
  • Switch from config 1 to config 2 while making a large number of requests to the archive engine webserver - in order to try to be in the for loop when the channels get changed over.

However this workflow didn't seem to be able to reproduce the exact error in the ticket.


To test:

  • Build this branch of CSS
  • Stop ibex server
  • Copy the .zips from isis_css_top\cs-studio\product\repository\target\products to EPICS\ICP_Binaries\css
  • Run setup_css.bat in EPICS\css\master
  • Restart IBEX server
  • Verify archivers still work as before (http://localhost:4813/groups and http://localhost:4812/groups)
  • Verify by code-review that the new code is thread-safe against the condition described above, and that you agree with my analysis that the stack trace in the ticket can only be caused by this type of threading condition.

@NikolaRoev NikolaRoev merged commit 364b5cb into master Dec 9, 2022
@NikolaRoev NikolaRoev deleted the Ticket7470_arblock_errors branch December 9, 2022 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants