From 2a967f5c99e58d81c93d7598cf4f7f4bdb3087d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Tue, 8 Apr 2025 15:48:30 +0200 Subject: [PATCH] Use __weak for implementation of QObjCWeakPointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By moving the implementation to qcore_mac.mm, and using a union for the object pointer, we can build qcore_mac.mm with -fobjc-weak to take advantage of the automatic weak-tracking. This allows us to drop the manual handling via ObjC associated objects, while also allowing non-Objective-C code to use QObjCWeakPointer. In particular we want to use it for QMacKeyValueObserver, which today runs the risk of removing the observation on an object that is long gone. Change-Id: I5d605e5ac82b39223b246d6758d0da88a1702357 Reviewed-by: Volker Hilsheimer Reviewed-by: Thiago Macieira (cherry picked from commit 1c98b3fe403aaa7b57813070f3bd690af7bd4e43) Reviewed-by: Tor Arne Vestbø --- src/corelib/CMakeLists.txt | 8 ++ src/corelib/kernel/qcore_mac.mm | 40 +++++++--- src/corelib/kernel/qcore_mac_p.h | 115 ++++++++--------------------- src/tools/bootstrap/CMakeLists.txt | 9 +++ 4 files changed, 77 insertions(+), 95 deletions(-) diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index 064dba4efae..8feb8bce81c 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -652,14 +652,22 @@ qt_internal_extend_target(Core CONDITION APPLE # - Provide DT_* macros to qfilesystemengine_unix.cpp # - Enables SOCK_MAXADDRLEN in case its missing during the unity build NO_UNITY_BUILD_SOURCES + kernel/qcore_mac.mm # See below kernel/qsystemerror.cpp # This makes sure that the tst_qmakelib passes. For some reason, # QtCore ends up returning a corrupted error message in # write_file(): fail + NO_PCH_SOURCES + kernel/qcore_mac.mm # See below ATTRIBUTION_FILE_DIR_PATHS kernel ) +if(APPLE) + # For QObjCWeakPointer + set_source_files_properties(kernel/qcore_mac.mm PROPERTIES COMPILE_FLAGS -fobjc-weak) +endif() + qt_internal_extend_target(Core CONDITION MACOS LIBRARIES ${FWAppKit} diff --git a/src/corelib/kernel/qcore_mac.mm b/src/corelib/kernel/qcore_mac.mm index 8b07838f2b3..e3641e25e80 100644 --- a/src/corelib/kernel/qcore_mac.mm +++ b/src/corelib/kernel/qcore_mac.mm @@ -774,17 +774,39 @@ QMacVersion::VersionTuple QMacVersion::libraryVersion() // ------------------------------------------------------------------------- -#if !(__has_feature(objc_arc_weak) && __has_feature(objc_arc_fields)) -QT_END_NAMESPACE -@implementation QT_MANGLE_NAMESPACE(WeakPointerLifetimeTracker) -- (void)dealloc +QObjCWeakPointerBase::QObjCWeakPointerBase(NSObject *object) + : m_weakReference(object) { - *self.pointer = {}; - [super dealloc]; } -@end -QT_BEGIN_NAMESPACE -#endif + +QObjCWeakPointerBase::QObjCWeakPointerBase(const QObjCWeakPointerBase &other) +{ + QMacAutoReleasePool pool; + m_weakReference = other.m_weakReference; +} + +QObjCWeakPointerBase &QObjCWeakPointerBase::operator=(const QObjCWeakPointerBase &other) +{ + QMacAutoReleasePool pool; + m_weakReference = other.m_weakReference; + return *this; +} + +QObjCWeakPointerBase::~QObjCWeakPointerBase() +{ + QMacAutoReleasePool pool; + m_weakReference = nil; +} + +NSObject *QObjCWeakPointerBase::get() const +{ + // Loading from a __weak variable will retain and auto-release (in non-ARC). + // Unlike cases above, we want the object to stay alive until the outer + // auto-release pool is drained, so that consumers of QObjCWeakPointer + // can trust that the variable they get back will be alive, similar to + // the semantics of loading from __weak. + return m_weakReference; +} // ------------------------------------------------------------------------- diff --git a/src/corelib/kernel/qcore_mac_p.h b/src/corelib/kernel/qcore_mac_p.h index 51efe0fe328..2d3d6d9c5c9 100644 --- a/src/corelib/kernel/qcore_mac_p.h +++ b/src/corelib/kernel/qcore_mac_p.h @@ -184,6 +184,35 @@ private: QString string; }; +class Q_CORE_EXPORT QObjCWeakPointerBase +{ +public: + QObjCWeakPointerBase(NSObject *object = nil); + QObjCWeakPointerBase(const QObjCWeakPointerBase &other); + QObjCWeakPointerBase &operator=(const QObjCWeakPointerBase &other); + +protected: + ~QObjCWeakPointerBase(); + NSObject *get() const; + union { + NSObject *m_object = nil; +#if __has_feature(objc_arc_weak) && __has_feature(objc_arc_fields) + // Used by qcore_mac.mm, built with -fobjc-weak, to track lifetime + __weak id m_weakReference; +#endif + }; +}; + +template +class QObjCWeakPointer : public QObjCWeakPointerBase +{ +public: + using QObjCWeakPointerBase::QObjCWeakPointerBase; + operator T*() const { return static_cast(get()); } +}; + +// ------------------------------------------------------------------------- + #ifdef Q_OS_MACOS Q_CORE_EXPORT bool qt_mac_applicationIsInDarkMode(); Q_CORE_EXPORT bool qt_mac_runningUnderRosetta(); @@ -462,92 +491,6 @@ qt_objc_cast(id object) // ------------------------------------------------------------------------- -#if defined( __OBJC__) - -template -class QObjCWeakPointer; - -#if __has_feature(objc_arc_weak) && __has_feature(objc_arc_fields) -# define USE_OBJC_WEAK 1 -#endif - -#if !USE_OBJC_WEAK -QT_END_NAMESPACE -#include -Q_CORE_EXPORT -QT_DECLARE_NAMESPACED_OBJC_INTERFACE(WeakPointerLifetimeTracker, NSObject -@property (atomic, assign) QT_PREPEND_NAMESPACE(QObjCWeakPointer) *pointer; -) -QT_BEGIN_NAMESPACE -#endif - -template -class QObjCWeakPointer -{ -public: - QObjCWeakPointer(T *object = nil) : m_object(object) - { -#if !USE_OBJC_WEAK - trackObjectLifetime(); -#endif - } - - QObjCWeakPointer(const QObjCWeakPointer &other) - { - QMacAutoReleasePool pool; - m_object = other.m_object; -#if !USE_OBJC_WEAK - trackObjectLifetime(); -#endif - } - - QObjCWeakPointer &operator=(const QObjCWeakPointer &other) - { - QMacAutoReleasePool pool; -#if !USE_OBJC_WEAK - objc_setAssociatedObject(m_object, this, nil, OBJC_ASSOCIATION_RETAIN); -#endif - m_object = other.m_object; -#if !USE_OBJC_WEAK - trackObjectLifetime(); -#endif - return *this; - } - - ~QObjCWeakPointer() - { -#if !USE_OBJC_WEAK - if (m_object) - objc_setAssociatedObject(m_object, this, nil, OBJC_ASSOCIATION_RETAIN); -#endif - } - - operator T*() const { return static_cast([[m_object retain] autorelease]); } - -private: -#if USE_OBJC_WEAK - __weak -#else - void trackObjectLifetime() - { - if (!m_object) - return; - - auto *lifetimeTracker = [WeakPointerLifetimeTracker new]; - lifetimeTracker.pointer = reinterpret_cast*>(this); - objc_setAssociatedObject(m_object, this, lifetimeTracker, OBJC_ASSOCIATION_RETAIN); - [lifetimeTracker release]; - } -#endif - NSObject *m_object = nil; -}; - -#undef USE_OBJC_WEAK - -#endif // __OBJC__ - -// ------------------------------------------------------------------------- - QT_END_NAMESPACE #endif // QCORE_MAC_P_H diff --git a/src/tools/bootstrap/CMakeLists.txt b/src/tools/bootstrap/CMakeLists.txt index 6a68425e873..802b1dc9504 100644 --- a/src/tools/bootstrap/CMakeLists.txt +++ b/src/tools/bootstrap/CMakeLists.txt @@ -143,10 +143,19 @@ qt_internal_extend_target(Bootstrap CONDITION APPLE ../../corelib/kernel/qcore_mac.mm ../../corelib/kernel/qcoreapplication_mac.cpp ../../corelib/tools/qversionnumber.cpp + NO_UNITY_BUILD_SOURCES + kernel/qcore_mac.mm # See below + NO_PCH_SOURCES + kernel/qcore_mac.mm # See below PUBLIC_LIBRARIES ${FWFoundation} ) +if(APPLE) + # For QObjCWeakPointer + set_source_files_properties(../../corelib/kernel/qcore_mac.mm PROPERTIES COMPILE_FLAGS -fobjc-weak) +endif() + qt_internal_extend_target(Bootstrap CONDITION MACOS SOURCES ../../corelib/io/qstandardpaths_mac.mm