Skip to content

Update Visualizer accesses to be safer and in the correct order to avoid race conditions#108

Open
bhung-bdai wants to merge 7 commits intorai-opensource:mainfrom
bhung-bdai:update-visualizer-to-be-safer
Open

Update Visualizer accesses to be safer and in the correct order to avoid race conditions#108
bhung-bdai wants to merge 7 commits intorai-opensource:mainfrom
bhung-bdai:update-visualizer-to-be-safer

Conversation

@bhung-bdai
Copy link
Copy Markdown
Collaborator

@bhung-bdai bhung-bdai commented Dec 12, 2025

The Visualizer class previously had a lot of issues. The two addressed in this PR are the following:

Traces being removed AFTER the geometries were removed, not before

Traces are attached to specific geometries. If a geometry is deleted before the trace is deleted, then the trace hangs around until the GUI is remade. This resulted in traces hanging around when you switched from a trace based geometry to a non-trace based geometry, as trace to trace switching would have bodies that the traces could reference before deleting.

This removes the issue by moving the trace removal before the geometry removal

Testing done

  • Switching from cylinder_push to caltech_leap_cube without seeing traces.

Events being recreated when a new task was created

Events and locks were created when a new task was set. This was bad, because you could potentially listen to an event in another process and have it be changed when you set a new task. This results in the event waiting forever, since the new event was not registered to the old listener.

This solves the issue by creating the events once in the Visualizer constructor.

Testing done

  • Ensure that task switching with another middleware implementation works and doesn't wait forever

Reorder how we set events

task_update.set and optimizer_updated.set were being done in the wrong locations. They were done as soon as the event was received, instead of after the Visualizer had finished processing. If using an async middleware, this might have resulted in the optimizer_updated or task_updated event response getting processed before the Visualizer had set up the task properly. In my instance, this resulted in weird viser body geometry states being carried over from a previous instance to the new instance and caused rendering issues.

Testing done

The solution is to set the event only after the Visualizer is done processing.

  • Switching from caltech_leap_cube to cylinder_push without seeing weird carryover issues in the rendering process.

@slecleach
Copy link
Copy Markdown
Collaborator

I tested it locally on my laptop, I couldn't break the app even with repeated changes of tasks, configs, controller, etc.
I didn't see trace issues when switching tasks.

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