Reintroduce the useage of FBConfigInfo* hash in glxbackend

Review Request #127096 - Created Feb. 17, 2016 and submitted

Information
Martin Flöser
kwin
master
Reviewers
kwin
kfunk, luebking
The hash was completely useless as we never inserted created
FBConfigInfo* into the hash. This change fixes the usage of the
hash and also ensures that it doesn't leak by deleting all hash
elements on tear down.

Current memleak backtrace from ASAN:
Direct leak of 48 byte(s) in 2 object(s) allocated from:
    #0 0x4dc932 in operator new(unsigned long) (/home/kfunk/devel/install/kf5/bin/kwin_x11+0x4dc932)
    #1 0x7f3c596de0f3 in KWin::GlxBackend::infoForVisual(unsigned int) /home/kfunk/devel/src/kf5/kwin/glxbackend.cpp:454:12
    #2 0x7f3c596e7ef4 in KWin::GlxTexture::loadTexture(unsigned int, QSize const&, unsigned int) /home/kfunk/devel/src/kf5/kwin/glxbackend.cpp:826:32
    #3 0x7f3c596e921e in KWin::GlxTexture::loadTexture(KWin::WindowPixmap*) /home/kfunk/devel/src/kf5/kwin/glxbackend.cpp:869:12
    #4 0x7f3c5916e895 in KWin::SceneOpenGL::Texture::load(KWin::WindowPixmap*) /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1232:12
    #5 0x7f3c5917019a in KWin::OpenGLWindowPixmap::bind() /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1681:20
    #6 0x7f3c5916fce0 in KWin::SceneOpenGL::Window::bindTexture() /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1288:12
    #7 0x7f3c5917294b in KWin::SceneOpenGL::Window::beginRenderWindow(int, QRegion const&, KWin::WindowPaintData&) /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1349:10
    #8 0x7f3c59176f24 in KWin::SceneOpenGL2Window::performPaint(int, QRegion, KWin::WindowPaintData) /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1510:10
    #9 0x7f3c5916d410 in KWin::SceneOpenGL2::performPaintWindow(KWin::EffectWindowImpl*, int, QRegion, KWin::WindowPaintData&) /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1161:9
    #10 0x7f3c5916cbc9 in KWin::SceneOpenGL2::finalDrawWindow(KWin::EffectWindowImpl*, int, QRegion, KWin::WindowPaintData&) /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1147:9
    #11 0x7f3c5922e92f in KWin::EffectsHandlerImpl::drawWindow(KWin::EffectWindow*, int, QRegion, KWin::WindowPaintData&) /home/kfunk/devel/src/kf5/kwin/effects.cpp:502:9
    #12 0x7f3c590d7bc5 in KWin::Scene::finalPaintWindow(KWin::EffectWindowImpl*, int, QRegion, KWin::WindowPaintData&) /home/kfunk/devel/src/kf5/kwin/scene.cpp:606:5
    #13 0x7f3c5922c979 in KWin::EffectsHandlerImpl::paintWindow(KWin::EffectWindow*, int, QRegion, KWin::WindowPaintData&) /home/kfunk/devel/src/kf5/kwin/effects.cpp:465:9
    #14 0x7f3c590d2b38 in KWin::Scene::paintWindow(KWin::Scene::Window*, int, QRegion, KWin::WindowQuadList) /home/kfunk/devel/src/kf5/kwin/scene.cpp:478:5
    #15 0x7f3c590ced89 in KWin::Scene::paintSimpleScreen(int, QRegion) /home/kfunk/devel/src/kf5/kwin/scene.cpp:381:9
    #16 0x7f3c5916b2b1 in KWin::SceneOpenGL2::paintSimpleScreen(int, QRegion) /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:1098:12
    #17 0x7f3c590c8bd3 in KWin::Scene::finalPaintScreen(int, QRegion, KWin::ScreenPaintData&) /home/kfunk/devel/src/kf5/kwin/scene.cpp:200:9
    #18 0x7f3c5922b062 in KWin::EffectsHandlerImpl::paintScreen(int, QRegion, KWin::ScreenPaintData&) /home/kfunk/devel/src/kf5/kwin/effects.cpp:422:9
    #19 0x7f3c590c791f in KWin::Scene::paintScreen(int*, QRegion const&, QRegion const&, QRegion*, QRegion*, QMatrix4x4 const&) /home/kfunk/devel/src/kf5/kwin/scene.cpp:150:5
    #20 0x7f3c5915e500 in KWin::SceneOpenGL::paint(QRegion, QList<KWin::Toplevel*>) /home/kfunk/devel/src/kf5/kwin/scene_opengl.cpp:751:9
    #21 0x7f3c5907ea3c in KWin::Compositor::performCompositing() /home/kfunk/devel/src/kf5/kwin/composite.cpp:726:29
    #22 0x7f3c5907c230 in KWin::Compositor::startupWithWorkspace() /home/kfunk/devel/src/kf5/kwin/composite.cpp:347:5
    #23 0x7f3c59079bbb in KWin::Compositor::slotCompositingOptionsInitialized() /home/kfunk/devel/src/kf5/kwin/composite.cpp:283:9
    #24 0x7f3c59076084 in KWin::Compositor::setup() /home/kfunk/devel/src/kf5/kwin/composite.cpp:184:9
    #25 0x7f3c598b165f in KWin::Compositor::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) /home/kfunk/devel/build/kf5/kwin/moc_composite.cpp:263:18
    #26 0x7f3c52de67b0 in QObject::event(QEvent*) (/usr/lib/x86_64-linux-gnu/libQt5Core.so.5+0x2b67b0)
    #27 0x7f3c536ab9db in QApplicationPrivate::notify_helper(QObject*, QEvent*) (/usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5+0x15b9db)

Not yet verified whether the memleak is truly fixed.

Issues

  • 1
  • 1
  • 0
  • 2
Description From Last Updated
std::unordered_map::operator[] returns a reference. Fredrik Höglund Fredrik Höglund
Thomas Lübking
Kevin Funk
Martin Flöser
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted with commit 36facda1107688a265b6f8edb9851c80a6b091cc by Martin Gräßlin to branch master.
Fredrik Höglund

   
glxbackend.cpp (Diff revision 1)
 
 

std::unordered_map::operator[] returns a reference.

  1. Yes, the particular code is correct (ie. the hash was in use)
    Overmore in the particular case there's no overhead because we're operating on a rather local member (so ::detach() is NOOP in this case) and indeed the unconditional addition is wanted.

    Technically, the old code was absolutely ok.


    Prettyprettyplease still avoid it anyway or (in case you prefer) add positive "i know what I'm doing" (or just "// fredrik" ;-) comments.
    The operator is one giant trap that caused us tons of headaches in the past (ppl. being "smart" to use a reference return to avoid copying the returned value, running into the operators trap)

    We really had dozens (that's not exaggerating) of QHash::operator[] mis-uses which lead to funky behavior everywhere - so everytime it pops up, it now rings an alarm :-(

Loading...