Use smart Refs for IOKit and CoreFoundation objects.
Bug: dawn:89
Change-Id: Idea634bcc65ab4ec017f4e4c2431e95915f533df
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/32661
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
diff --git a/src/common/BUILD.gn b/src/common/BUILD.gn
index 1e31801..698b26c 100644
--- a/src/common/BUILD.gn
+++ b/src/common/BUILD.gn
@@ -157,11 +157,13 @@
"BitSetIterator.h",
"Compiler.h",
"Constants.h",
+ "CoreFoundationRef.h",
"DynamicLib.cpp",
"DynamicLib.h",
"GPUInfo.cpp",
"GPUInfo.h",
"HashUtils.h",
+ "IOKitRef.h",
"LinkedList.h",
"Log.cpp",
"Log.h",
diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt
index 8ea48e0..86812af 100644
--- a/src/common/CMakeLists.txt
+++ b/src/common/CMakeLists.txt
@@ -19,11 +19,13 @@
"BitSetIterator.h"
"Compiler.h"
"Constants.h"
+ "CoreFoundationRef.h"
"DynamicLib.cpp"
"DynamicLib.h"
"GPUInfo.cpp"
"GPUInfo.h"
"HashUtils.h"
+ "IOKitRef.h"
"LinkedList.h"
"Log.cpp"
"Log.h"
diff --git a/src/common/CoreFoundationRef.h b/src/common/CoreFoundationRef.h
new file mode 100644
index 0000000..a4a637b
--- /dev/null
+++ b/src/common/CoreFoundationRef.h
@@ -0,0 +1,46 @@
+// Copyright 2020 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef COMMON_COREFOUNDATIONREF_H_
+#define COMMON_COREFOUNDATIONREF_H_
+
+#include "common/RefBase.h"
+
+#include <CoreFoundation/CoreFoundation.h>
+
+template <typename T>
+struct CoreFoundationRefTraits {
+ static constexpr T kNullValue = nullptr;
+ static void Reference(T value) {
+ CFRetain(value);
+ }
+ static void Release(T value) {
+ CFRelease(value);
+ }
+};
+
+template <typename T>
+class CFRef : public RefBase<T, CoreFoundationRefTraits<T>> {
+ public:
+ using RefBase<T, CoreFoundationRefTraits<T>>::RefBase;
+};
+
+template <typename T>
+CFRef<T> AcquireCFRef(T pointee) {
+ CFRef<T> ref;
+ ref.Acquire(pointee);
+ return ref;
+}
+
+#endif // COMMON_COREFOUNDATIONREF_H_
diff --git a/src/common/IOKitRef.h b/src/common/IOKitRef.h
new file mode 100644
index 0000000..cba0376
--- /dev/null
+++ b/src/common/IOKitRef.h
@@ -0,0 +1,46 @@
+// Copyright 2020 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef COMMON_IOKITREF_H_
+#define COMMON_IOKITREF_H_
+
+#include "common/RefBase.h"
+
+#include <IOKit/IOKitLib.h>
+
+template <typename T>
+struct IOKitRefTraits {
+ static constexpr T kNullValue = IO_OBJECT_NULL;
+ static void Reference(T value) {
+ IOObjectRetain(value);
+ }
+ static void Release(T value) {
+ IOObjectRelease(value);
+ }
+};
+
+template <typename T>
+class IORef : public RefBase<T, IOKitRefTraits<T>> {
+ public:
+ using RefBase<T, IOKitRefTraits<T>>::RefBase;
+};
+
+template <typename T>
+IORef<T> AcquireIORef(T pointee) {
+ IORef<T> ref;
+ ref.Acquire(pointee);
+ return ref;
+}
+
+#endif // COMMON_IOKITREF_H_
diff --git a/src/common/RefBase.h b/src/common/RefBase.h
index 508b424..a1db6a9 100644
--- a/src/common/RefBase.h
+++ b/src/common/RefBase.h
@@ -15,6 +15,7 @@
#ifndef COMMON_REFBASE_H_
#define COMMON_REFBASE_H_
+#include "common/Assert.h"
#include "common/Compiler.h"
#include <type_traits>
@@ -163,6 +164,11 @@
mValue = value;
}
+ T* InitializeInto() DAWN_NO_DISCARD {
+ ASSERT(mValue == kNullValue);
+ return &mValue;
+ }
+
private:
// Friend is needed so that instances of RefBase<U> can call Reference and Release on
// RefBase<T>.
diff --git a/src/dawn_native/metal/BackendMTL.mm b/src/dawn_native/metal/BackendMTL.mm
index f7b1742..ada2c6d 100644
--- a/src/dawn_native/metal/BackendMTL.mm
+++ b/src/dawn_native/metal/BackendMTL.mm
@@ -14,6 +14,7 @@
#include "dawn_native/metal/BackendMTL.h"
+#include "common/CoreFoundationRef.h"
#include "common/GPUInfo.h"
#include "common/NSRef.h"
#include "common/Platform.h"
@@ -23,6 +24,7 @@
#if defined(DAWN_PLATFORM_MACOS)
# import <IOKit/IOKitLib.h>
+# include "common/IOKitRef.h"
#endif
namespace dawn_native { namespace metal {
@@ -72,18 +74,17 @@
// Recursively search registry entry and its parents for property name
// The data should release with CFRelease
- CFDataRef data = static_cast<CFDataRef>(IORegistryEntrySearchCFProperty(
- entry, kIOServicePlane, name, kCFAllocatorDefault,
- kIORegistryIterateRecursively | kIORegistryIterateParents));
+ CFRef<CFDataRef> data =
+ AcquireCFRef(static_cast<CFDataRef>(IORegistryEntrySearchCFProperty(
+ entry, kIOServicePlane, name, kCFAllocatorDefault,
+ kIORegistryIterateRecursively | kIORegistryIterateParents)));
if (data == nullptr) {
return value;
}
// CFDataGetBytePtr() is guaranteed to return a read-only pointer
- value = *reinterpret_cast<const uint32_t*>(CFDataGetBytePtr(data));
-
- CFRelease(data);
+ value = *reinterpret_cast<const uint32_t*>(CFDataGetBytePtr(data.Get()));
return value;
}
@@ -107,38 +108,35 @@
MaybeError API_AVAILABLE(macos(10.13))
GetDeviceIORegistryPCIInfo(id<MTLDevice> device, PCIIDs* ids) {
// Get a matching dictionary for the IOGraphicsAccelerator2
- CFMutableDictionaryRef matchingDict = IORegistryEntryIDMatching([device registryID]);
+ CFRef<CFMutableDictionaryRef> matchingDict =
+ AcquireCFRef(IORegistryEntryIDMatching([device registryID]));
if (matchingDict == nullptr) {
return DAWN_INTERNAL_ERROR("Failed to create the matching dict for the device");
}
// IOServiceGetMatchingService will consume the reference on the matching dictionary,
// so we don't need to release the dictionary.
- io_registry_entry_t acceleratorEntry =
- IOServiceGetMatchingService(kIOMasterPortDefault, matchingDict);
+ IORef<io_registry_entry_t> acceleratorEntry = AcquireIORef(
+ IOServiceGetMatchingService(kIOMasterPortDefault, matchingDict.Detach()));
if (acceleratorEntry == IO_OBJECT_NULL) {
return DAWN_INTERNAL_ERROR(
"Failed to get the IO registry entry for the accelerator");
}
// Get the parent entry that will be the IOPCIDevice
- io_registry_entry_t deviceEntry = IO_OBJECT_NULL;
- if (IORegistryEntryGetParentEntry(acceleratorEntry, kIOServicePlane, &deviceEntry) !=
- kIOReturnSuccess) {
- IOObjectRelease(acceleratorEntry);
+ IORef<io_registry_entry_t> deviceEntry;
+ if (IORegistryEntryGetParentEntry(acceleratorEntry.Get(), kIOServicePlane,
+ deviceEntry.InitializeInto()) != kIOReturnSuccess) {
return DAWN_INTERNAL_ERROR("Failed to get the IO registry entry for the device");
}
ASSERT(deviceEntry != IO_OBJECT_NULL);
- IOObjectRelease(acceleratorEntry);
- uint32_t vendorId = GetEntryProperty(deviceEntry, CFSTR("vendor-id"));
- uint32_t deviceId = GetEntryProperty(deviceEntry, CFSTR("device-id"));
+ uint32_t vendorId = GetEntryProperty(deviceEntry.Get(), CFSTR("vendor-id"));
+ uint32_t deviceId = GetEntryProperty(deviceEntry.Get(), CFSTR("device-id"));
*ids = PCIIDs{vendorId, deviceId};
- IOObjectRelease(deviceEntry);
-
return {};
}
diff --git a/src/tests/unittests/RefCountedTests.cpp b/src/tests/unittests/RefCountedTests.cpp
index 5da3118..236dd16 100644
--- a/src/tests/unittests/RefCountedTests.cpp
+++ b/src/tests/unittests/RefCountedTests.cpp
@@ -402,3 +402,17 @@
destination = nullptr;
EXPECT_TRUE(deleted);
}
+
+// Test Ref's InitializeInto.
+TEST(Ref, InitializeInto) {
+ bool deleted = false;
+ RCTest* original = new RCTest(&deleted);
+
+ // InitializeInto acquires the ref.
+ Ref<RCTest> ref;
+ *ref.InitializeInto() = original;
+ EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
+ ref = nullptr;
+ EXPECT_TRUE(deleted);
+}
