Skip to content

Conversation

HomerSp
Copy link
Contributor

@HomerSp HomerSp commented Mar 9, 2022

Description

This fixes a bad sigsegv on AMD GPUs (maybe others too, though I've only seen it reported on AMD) running on Linux. Basically, because of the order of the members in session_t, hwdevice would be destroyed before the avcodec context. This causes problems because the context relies on the hwdevice still being active - as in, when the hwdevice is destroyed the dri library is unloaded, and va calls methods in there.

Here's a backtrace of the sigsegv:

#0  0x00007fffe6f39630 in  () -> (this is supposed to be si_resource_destroy in radeonsi_drv_video.so but because it's unloaded it no longer exists)
#1  0x00007fff7835dea1 in pipe_resource_destroy (res=<optimised out>)
#2  pipe_resource_reference (src=0x0, dst=0x5555578c0e88)
#3  vlVaDestroyBuffer (ctx=0x555556997b30, buf_id=5)
#4  0x00007ffff2224831 in vaDestroyBuffer (dpy=0x5555569979c0, buffer_id=buffer_id@entry=5)
#5  0x00007ffff6e78013 in vaapi_encode_free_output_buffer (opaque=0x5555563c3f00, data=0x5 <error: Cannot access memory
#6  0x00007ffff6631a5f in buffer_pool_free (pool=<optimised out>)
#7  av_buffer_pool_uninit (ppool=ppool@entry=0x555556a58c88)
#8  0x00007ffff680f656 in ff_vaapi_encode_close (avctx=0x5555563c3f00)
#9  0x00007ffff680dca2 in avcodec_close (avctx=avctx@entry=0x5555563c3f00)
#10 0x00007ffff6d2890d in avcodec_free_context (pavctx=0x7fffffffd288)
#11 0x0000555555dc5a97 in video::free_ctx(AVCodecContext*) (ctx=0x5555563c3f00)
#12 0x0000555555de1f3e in util::Destroy<AVCodecContext*, void, &video::free_ctx>::operator()(AVCodecContext*) (this=0x7fffffffd468, p=0x5555563c3f00)
#13 0x0000555555ddb5a2 in util::uniq_ptr<AVCodecContext, util::Destroy<AVCodecContext*, void, &video::free_ctx> >::reset(AVCodecContext*) (this=0x7fffffffd460, p=0x0)
   
#14 0x0000555555dd518f in util::uniq_ptr<AVCodecContext, util::Destroy<AVCodecContext*, void, &video::free_ctx> >::~uniq_ptr() (this=0x7fffffffd460, __in_chrg=<optimised out>)
   
#15 0x0000555555dd4792 in video::session_t::~session_t() (this=0x7fffffffd460, __in_chrg=<optimised out>)
#16 0x0000555555de4725 in std::_Optional_payload_base<video::session_t>::_M_destroy() (this=0x7fffffffd460)
#17 0x0000555555ddde5d in std::_Optional_payload_base<video::session_t>::_M_reset() (this=0x7fffffffd460)
#18 0x0000555555dd7600 in std::_Optional_payload<video::session_t, false, false, false>::~_Optional_payload() (this=0x7fffffffd460, __in_chrg=<optimised out>)
#19 0x0000555555dd428e in std::_Optional_base<video::session_t, false, false>::~_Optional_base() (this=0x7fffffffd460, __in_chrg=<optimised out>)
#20 0x0000555555dd42ae in std::optional<video::session_t>::~optional() (this=0x7fffffffd460, __in_chrg=<optimised out>)
#21 0x0000555555dcbecc in video::validate_config(std::shared_ptr<platf::display_t>&, video::encoder_t const&, video::config_t const&) (disp=
    std::shared_ptr<class platf::display_t> (use count 1, weak count 0) = {...}, encoder=..., config=...)
#22 0x0000555555dcc42b in video::validate_encoder(video::encoder_t&) (encoder=...)
#23 0x0000555555dcddab in video::init() ()
#24 0x0000555555c5c327 in main(int, char**) (argc=2, argv=0x7fffffffdac8)

Screenshot

Include screenshots if the changes are UI-related.

Issues Fixed or Closed

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the documentation blocks for new or existing components

…e the context relies on the hwdevice still being active.
@HomerSp HomerSp changed the title Fix hwdevice being destroyed before context, Fix hwdevice being destroyed before context causing sigsegv on AMD Mar 9, 2022
@HomerSp
Copy link
Contributor Author

HomerSp commented Mar 10, 2022

I've updated it slightly so the order of the members won't matter, instead the session_t destructor will ensure they're destroyed in the correct order.

@TheElixZammuto
Copy link
Member

I can't test this, but both code and rationale seems reasonable. LGTM

@ReenigneArcher ReenigneArcher merged commit ea27955 into LizardByte:nightly Mar 13, 2022
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