Skip to content

Commit add7285

Browse files
authored
portals: Fix use of a potentially destroyed session (#355)
1 parent 371b96b commit add7285

File tree

2 files changed

+67
-39
lines changed

2 files changed

+67
-39
lines changed

src/portals/Screencopy.cpp

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,14 @@ dbUasv CScreencopyPortal::onCreateSession(sdbus::ObjectPath requestHandle, sdbus
4141
Debug::log(LOG, "[screencopy] | {}", sessionHandle.c_str());
4242
Debug::log(LOG, "[screencopy] | appid: {}", appID);
4343

44-
const auto PSESSION = m_vSessions.emplace_back(std::make_unique<SSession>(appID, requestHandle, sessionHandle)).get();
44+
const Hyprutils::Memory::CWeakPointer<SSession> PSESSION = m_vSessions.emplace_back(Hyprutils::Memory::makeUnique<SSession>(appID, requestHandle, sessionHandle));
45+
PSESSION->self = PSESSION;
4546

4647
// create objects
4748
PSESSION->session = createDBusSession(sessionHandle);
4849
PSESSION->session->onDestroy = [PSESSION, this]() {
4950
if (PSESSION->sharingData.active) {
50-
m_pPipewire->destroyStream(PSESSION);
51+
m_pPipewire->destroyStream(PSESSION.get());
5152
Debug::log(LOG, "[screencopy] Stream destroyed");
5253
}
5354
PSESSION->session.release();
@@ -346,8 +347,10 @@ void CScreencopyPortal::SSession::startCopy() {
346347

347348
void CScreencopyPortal::SSession::initCallbacks() {
348349
if (sharingData.frameCallback) {
349-
sharingData.frameCallback->setBuffer([this](CCZwlrScreencopyFrameV1* r, uint32_t format, uint32_t width, uint32_t height, uint32_t stride) {
350-
Debug::log(TRACE, "[sc] wlrOnBuffer for {}", (void*)this);
350+
sharingData.frameCallback->setBuffer([this, self = self](CCZwlrScreencopyFrameV1* r, uint32_t format, uint32_t width, uint32_t height, uint32_t stride) {
351+
Debug::log(TRACE, "[sc] wlrOnBuffer for {}", (void*)self.get());
352+
if (!self)
353+
return;
351354

352355
sharingData.frameInfoSHM.w = width;
353356
sharingData.frameInfoSHM.h = height;
@@ -357,8 +360,10 @@ void CScreencopyPortal::SSession::initCallbacks() {
357360

358361
// todo: done if ver < 3
359362
});
360-
sharingData.frameCallback->setReady([this](CCZwlrScreencopyFrameV1* r, uint32_t tv_sec_hi, uint32_t tv_sec_lo, uint32_t tv_nsec) {
361-
Debug::log(TRACE, "[sc] wlrOnReady for {}", (void*)this);
363+
sharingData.frameCallback->setReady([this, self = self](CCZwlrScreencopyFrameV1* r, uint32_t tv_sec_hi, uint32_t tv_sec_lo, uint32_t tv_nsec) {
364+
Debug::log(TRACE, "[sc] wlrOnReady for {}", (void*)self.get());
365+
if (!self)
366+
return;
362367

363368
sharingData.status = FRAME_READY;
364369

@@ -375,12 +380,16 @@ void CScreencopyPortal::SSession::initCallbacks() {
375380

376381
sharingData.frameCallback.reset();
377382
});
378-
sharingData.frameCallback->setFailed([this](CCZwlrScreencopyFrameV1* r) {
379-
Debug::log(TRACE, "[sc] wlrOnFailed for {}", (void*)this);
383+
sharingData.frameCallback->setFailed([this, self = self](CCZwlrScreencopyFrameV1* r) {
384+
Debug::log(TRACE, "[sc] wlrOnFailed for {}", (void*)self.get());
385+
if (!self)
386+
return;
380387
sharingData.status = FRAME_FAILED;
381388
});
382-
sharingData.frameCallback->setDamage([this](CCZwlrScreencopyFrameV1* r, uint32_t x, uint32_t y, uint32_t width, uint32_t height) {
383-
Debug::log(TRACE, "[sc] wlrOnDamage for {}", (void*)this);
389+
sharingData.frameCallback->setDamage([this, self = self](CCZwlrScreencopyFrameV1* r, uint32_t x, uint32_t y, uint32_t width, uint32_t height) {
390+
Debug::log(TRACE, "[sc] wlrOnDamage for {}", (void*)self.get());
391+
if (!self)
392+
return;
384393

385394
if (sharingData.damageCount > 3) {
386395
sharingData.damage[0] = {0, 0, sharingData.frameInfoDMA.w, sharingData.frameInfoDMA.h};
@@ -391,15 +400,19 @@ void CScreencopyPortal::SSession::initCallbacks() {
391400

392401
Debug::log(TRACE, "[sc] wlr damage: {} {} {} {}", x, y, width, height);
393402
});
394-
sharingData.frameCallback->setLinuxDmabuf([this](CCZwlrScreencopyFrameV1* r, uint32_t format, uint32_t width, uint32_t height) {
395-
Debug::log(TRACE, "[sc] wlrOnDmabuf for {}", (void*)this);
403+
sharingData.frameCallback->setLinuxDmabuf([this, self = self](CCZwlrScreencopyFrameV1* r, uint32_t format, uint32_t width, uint32_t height) {
404+
Debug::log(TRACE, "[sc] wlrOnDmabuf for {}", (void*)self.get());
405+
if (!self)
406+
return;
396407

397408
sharingData.frameInfoDMA.w = width;
398409
sharingData.frameInfoDMA.h = height;
399410
sharingData.frameInfoDMA.fmt = format;
400411
});
401-
sharingData.frameCallback->setBufferDone([this](CCZwlrScreencopyFrameV1* r) {
402-
Debug::log(TRACE, "[sc] wlrOnBufferDone for {}", (void*)this);
412+
sharingData.frameCallback->setBufferDone([this, self = self](CCZwlrScreencopyFrameV1* r) {
413+
Debug::log(TRACE, "[sc] wlrOnBufferDone for {}", (void*)self.get());
414+
if (!self)
415+
return;
403416

404417
const auto PSTREAM = g_pPortalManager->m_sPortals.screencopy->m_pPipewire->streamFromSession(this);
405418

@@ -449,8 +462,10 @@ void CScreencopyPortal::SSession::initCallbacks() {
449462
Debug::log(TRACE, "[sc] wlr frame copied");
450463
});
451464
} else if (sharingData.windowFrameCallback) {
452-
sharingData.windowFrameCallback->setBuffer([this](CCHyprlandToplevelExportFrameV1* r, uint32_t format, uint32_t width, uint32_t height, uint32_t stride) {
453-
Debug::log(TRACE, "[sc] hlOnBuffer for {}", (void*)this);
465+
sharingData.windowFrameCallback->setBuffer([this, self = self](CCHyprlandToplevelExportFrameV1* r, uint32_t format, uint32_t width, uint32_t height, uint32_t stride) {
466+
Debug::log(TRACE, "[sc] hlOnBuffer for {}", (void*)self.get());
467+
if (!self)
468+
return;
454469

455470
sharingData.frameInfoSHM.w = width;
456471
sharingData.frameInfoSHM.h = height;
@@ -460,8 +475,10 @@ void CScreencopyPortal::SSession::initCallbacks() {
460475

461476
// todo: done if ver < 3
462477
});
463-
sharingData.windowFrameCallback->setReady([this](CCHyprlandToplevelExportFrameV1* r, uint32_t tv_sec_hi, uint32_t tv_sec_lo, uint32_t tv_nsec) {
464-
Debug::log(TRACE, "[sc] hlOnReady for {}", (void*)this);
478+
sharingData.windowFrameCallback->setReady([this, self = self](CCHyprlandToplevelExportFrameV1* r, uint32_t tv_sec_hi, uint32_t tv_sec_lo, uint32_t tv_nsec) {
479+
Debug::log(TRACE, "[sc] hlOnReady for {}", (void*)self.get());
480+
if (!self)
481+
return;
465482

466483
sharingData.status = FRAME_READY;
467484

@@ -478,12 +495,16 @@ void CScreencopyPortal::SSession::initCallbacks() {
478495

479496
sharingData.windowFrameCallback.reset();
480497
});
481-
sharingData.windowFrameCallback->setFailed([this](CCHyprlandToplevelExportFrameV1* r) {
482-
Debug::log(TRACE, "[sc] hlOnFailed for {}", (void*)this);
498+
sharingData.windowFrameCallback->setFailed([this, self = self](CCHyprlandToplevelExportFrameV1* r) {
499+
Debug::log(TRACE, "[sc] hlOnFailed for {}", (void*)self.get());
500+
if (!self)
501+
return;
483502
sharingData.status = FRAME_FAILED;
484503
});
485-
sharingData.windowFrameCallback->setDamage([this](CCHyprlandToplevelExportFrameV1* r, uint32_t x, uint32_t y, uint32_t width, uint32_t height) {
486-
Debug::log(TRACE, "[sc] hlOnDamage for {}", (void*)this);
504+
sharingData.windowFrameCallback->setDamage([this, self = self](CCHyprlandToplevelExportFrameV1* r, uint32_t x, uint32_t y, uint32_t width, uint32_t height) {
505+
Debug::log(TRACE, "[sc] hlOnDamage for {}", (void*)self.get());
506+
if (!self)
507+
return;
487508

488509
if (sharingData.damageCount > 3) {
489510
sharingData.damage[0] = {0, 0, sharingData.frameInfoDMA.w, sharingData.frameInfoDMA.h};
@@ -494,15 +515,19 @@ void CScreencopyPortal::SSession::initCallbacks() {
494515

495516
Debug::log(TRACE, "[sc] hl damage: {} {} {} {}", x, y, width, height);
496517
});
497-
sharingData.windowFrameCallback->setLinuxDmabuf([this](CCHyprlandToplevelExportFrameV1* r, uint32_t format, uint32_t width, uint32_t height) {
498-
Debug::log(TRACE, "[sc] hlOnDmabuf for {}", (void*)this);
518+
sharingData.windowFrameCallback->setLinuxDmabuf([this, self = self](CCHyprlandToplevelExportFrameV1* r, uint32_t format, uint32_t width, uint32_t height) {
519+
Debug::log(TRACE, "[sc] hlOnDmabuf for {}", (void*)self.get());
520+
if (!self)
521+
return;
499522

500523
sharingData.frameInfoDMA.w = width;
501524
sharingData.frameInfoDMA.h = height;
502525
sharingData.frameInfoDMA.fmt = format;
503526
});
504-
sharingData.windowFrameCallback->setBufferDone([this](CCHyprlandToplevelExportFrameV1* r) {
505-
Debug::log(TRACE, "[sc] hlOnBufferDone for {}", (void*)this);
527+
sharingData.windowFrameCallback->setBufferDone([this, self = self](CCHyprlandToplevelExportFrameV1* r) {
528+
Debug::log(TRACE, "[sc] hlOnBufferDone for {}", (void*)self.get());
529+
if (!self)
530+
return;
506531

507532
const auto PSTREAM = g_pPortalManager->m_sPortals.screencopy->m_pPipewire->streamFromSession(this);
508533

src/portals/Screencopy.hpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
#include "wlr-screencopy-unstable-v1.hpp"
44
#include "hyprland-toplevel-export-v1.hpp"
5+
#include <hyprutils/memory/UniquePtr.hpp>
6+
#include <hyprutils/memory/WeakPtr.hpp>
57
#include <sdbus-c++/sdbus-c++.h>
68
#include "../shared/ScreencopyShared.hpp"
79
#include <gbm.h>
@@ -62,17 +64,18 @@ class CScreencopyPortal {
6264
std::unordered_map<std::string, sdbus::Variant> opts);
6365

6466
struct SSession {
65-
std::string appid;
66-
sdbus::ObjectPath requestHandle, sessionHandle;
67-
uint32_t cursorMode = HIDDEN;
68-
uint32_t persistMode = 0;
67+
std::string appid;
68+
sdbus::ObjectPath requestHandle, sessionHandle;
69+
uint32_t cursorMode = HIDDEN;
70+
uint32_t persistMode = 0;
6971

70-
std::unique_ptr<SDBusRequest> request;
71-
std::unique_ptr<SDBusSession> session;
72-
SSelectionData selection;
72+
std::unique_ptr<SDBusRequest> request;
73+
std::unique_ptr<SDBusSession> session;
74+
SSelectionData selection;
75+
Hyprutils::Memory::CWeakPointer<SSession> self;
7376

74-
void startCopy();
75-
void initCallbacks();
77+
void startCopy();
78+
void initCallbacks();
7679

7780
struct {
7881
bool active = false;
@@ -113,12 +116,12 @@ class CScreencopyPortal {
113116
std::unique_ptr<CPipewireConnection> m_pPipewire;
114117

115118
private:
116-
std::unique_ptr<sdbus::IObject> m_pObject;
119+
std::unique_ptr<sdbus::IObject> m_pObject;
117120

118-
std::vector<std::unique_ptr<SSession>> m_vSessions;
121+
std::vector<Hyprutils::Memory::CUniquePointer<SSession>> m_vSessions;
119122

120-
SSession* getSession(sdbus::ObjectPath& path);
121-
void startSharing(SSession* pSession);
123+
SSession* getSession(sdbus::ObjectPath& path);
124+
void startSharing(SSession* pSession);
122125

123126
struct {
124127
SP<CCZwlrScreencopyManagerV1> screencopy = nullptr;

0 commit comments

Comments
 (0)