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

fix: double render call with timeline obj causes a seg fault #706

Merged

Conversation

devmessias
Copy link
Contributor

@devmessias devmessias commented Nov 12, 2022

This closes #705

Description

test_morphing

def test_morphing():
causes a segmentation fault error in most of the github actions. It seems correlated with the headless mode.

  • The double call Render method with a timeline and a gltf object in a headless env seems to be the cause of the seg fault.

I've created a simple repo with just the test_morphing to debug this issue. The following zip has all the logs of the failed actions:
12_DEBUG fail-test.txt : catchsegv python3 debug_fail.py
13_DEBUG catch.txt: python3 -X faulthandler debug_fail.py

logs_github_actions_traceback.zip

@codecov
Copy link

codecov bot commented Nov 12, 2022

Codecov Report

Merging #706 (af9dc79) into master (e0f1613) will increase coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #706      +/-   ##
==========================================
+ Coverage   50.14%   50.19%   +0.05%     
==========================================
  Files         120      120              
  Lines       27884    28019     +135     
  Branches     2961     2987      +26     
==========================================
+ Hits        13982    14064      +82     
- Misses      13436    13489      +53     
  Partials      466      466              
Impacted Files Coverage Δ
fury/gltf.py 0.00% <0.00%> (ø)
fury/fury/animation/timeline.py 66.08% <0.00%> (+0.21%) ⬆️
fury/fury/tests/test_gltf.py 89.44% <0.00%> (+1.49%) ⬆️
fury/fury/gltf.py 80.33% <0.00%> (+1.61%) ⬆️

Copy link
Contributor

@filipinascimento filipinascimento left a comment

Choose a reason for hiding this comment

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

VTK should support this functionally. Users should be able to render and after that save a screenshot. This however seems to fix the seg fault. We may need to create a issue and a minimal example to fully understand what is happening. The logs and tracing of the issue do not help much unfortunately, except to point to this line as the problem.

I think this can be merged, just to have these problems fixed.

@devmessias
Copy link
Contributor Author

Maybe there is a issue with mesa and vtk? Also, this render has no meaning there, because the buffer continue to be updated.

@m-agour
Copy link
Contributor

m-agour commented Nov 15, 2022

@devmessias @filipinascimento we actually will not need any showm.render() since record function creates a new iren and a new RenderWindow. so force rendering will not help the screenshot in any way. The problem lies with in the showManager, something is not being created or cleaned properly.
I think best way to debug it is to find out why the minimal example here #603 (comment) is failing.

@devmessias
Copy link
Contributor Author

devmessias commented Nov 15, 2022

I looked at some tests being skipped because of SEG FAULT. I believe we can have multiple causes for different SEG FAULTs. Some of them happens only in the headless mode. Besides that, we have some PRs that can help to mitigate this. For example, we have an open issue with timer callbacks that are not properly being removed.
#483

@devmessias
Copy link
Contributor Author

devmessias commented Nov 15, 2022

Hi @m-agour are you talking about this ? image

The problem here is that you never removed the context, events etc from the second show_manager instance.

This code works without SEG FAULTs

import os
from fury import window, ui
from fury.data import DATA_DIR

def test_1():
    scene = window.Scene()
    show_manager = window.ShowManager(scene)
    show_manager.initialize()
    show_manager.start()
    filename = "test_ui_file_menu_2d"
    recording_filename = os.path.join(DATA_DIR, filename + ".log.gz")

    show_manager = window.ShowManager(size=(600, 600), title="FURY FileMenu")
    show_manager.play_events_from_file(recording_filename)
    
    show_manager.window.RemoveRenderer(show_manager.scene)
    show_manager.scene.SetRenderWindow(None)
    show_manager.window.Finalize()
    del show_manager.iren
    del show_manager.window

    filemenu = ui.FileMenu2D(size=(500, 500),
                             directory_path=os.getcwd())
    show_manager = window.ShowManager(size=(600, 600),
                                      title="FURY FileMenu")
    show_manager.scene.add(filemenu)
    show_manager.start()


test_1()

@m-agour
Copy link
Contributor

m-agour commented Nov 17, 2022

@devmessias Yes this the example. It only causes a seg fault in Ubuntu (mostly in all linux distros)
Your modification is still giving me a seg fault at the last line show_manager.start() in Ubuntu. Even running it as a normal code it still gives a sig fault Process finished with exit code 139 (interrupted by signal 11: SIGSEGV)

@Garyfallidis
Copy link
Contributor

This is a single line change. So will merge this and be happy to continue the segfault discussion here or another PR.

@Garyfallidis Garyfallidis merged commit 8e1cc27 into fury-gl:master Nov 17, 2022
@devmessias
Copy link
Contributor Author

m-agour

@devmessias Yes this the example. It only causes a seg fault in Ubuntu (mostly in all linux distros) Your modification is still giving me a seg fault at the last line show_manager.start() in Ubuntu. Even running it as a normal code it still gives a sig fault Process finished with exit code 139 (interrupted by signal 11: SIGSEGV)

My mistake, I was using my custom window module to run your example @m-agour , I'll sent a PR this week with those changes.

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.

[BUG] Segmentation fault error caused by Morph Stress Test
4 participants