fix: fix access race in the network module (#485)

Modules/Disk/main.swift

@@ -19,13 +19,9 @@ public struct stats {
     
     var readBytes: Int64 = 0
     var writeBytes: Int64 = 0
-    var readOperations: Int64 = 0
-    var writeOperations: Int64 = 0
-    var readTime: Int64 = 0
-    var writeTime: Int64 = 0
 }
 
-struct drive {
+public struct drive {
     var parent: io_registry_entry_t = 0
     
     var mediaName: String = ""
@@ -45,50 +41,92 @@ struct drive {
     var activity: stats = stats()
 }
 
-struct DiskList: value_t {
-    var list: [drive] = []
+public class Disks {
+    fileprivate let queue = DispatchQueue(label: "eu.exelban.Stats.Disk.SynchronizedArray", attributes: .concurrent)
+    fileprivate var array = [drive]()
     
-    public var widget_value: Double {
-        get {
-            return 0
+    public var count: Int {
+        var result = 0
+        self.queue.sync { result = self.array.count }
+        return result
+    }
+    
+    public func first(where predicate: (drive) -> Bool) -> drive? {
+        var result: drive?
+        self.queue.sync { result = self.array.first(where: predicate) }
+        return result
+    }
+    
+    public func index(where predicate: (drive) -> Bool) -> Int? {
+        var result: Int?
+        self.queue.sync { result = self.array.firstIndex(where: predicate) }
+        return result
+    }
+    
+    public func map<ElementOfResult>(_ transform: (drive) -> ElementOfResult?) -> [ElementOfResult] {
+        var result = [ElementOfResult]()
+        self.queue.sync { result = self.array.compactMap(transform) }
+        return result
+    }
+    
+    public func reversed() -> [drive] {
+        var result: [drive] = []
+        self.queue.sync(flags: .barrier) { result = self.array.reversed() }
+        return result
+    }
+    
+    func forEach(_ body: (drive) -> Void) {
+        self.queue.sync { self.array.forEach(body) }
+    }
+    
+    public func append( _ element: drive) {
+        self.queue.async(flags: .barrier) {
+            self.array.append(element)
         }
     }
     
-    func getDiskByBSDName(_ name: String) -> drive? {
-        if let idx = self.list.firstIndex(where: { $0.BSDName == name }) {
-            return self.list[idx]
+    public func remove(at index: Int) {
+        self.queue.async(flags: .barrier) {
+            self.array.remove(at: index)
         }
-        
-        return nil
     }
     
-    func getDiskByName(_ name: String) -> drive? {
-        if let idx = self.list.firstIndex(where: { $0.mediaName == name }) {
-            return self.list[idx]
+    public func sort() {
+        self.queue.async(flags: .barrier) {
+            self.array.sort{ $1.removable }
         }
-        
-        return nil
     }
     
-    func getRootDisk() -> drive? {
-        if let idx = self.list.firstIndex(where: { $0.root }) {
-            return self.list[idx]
+    func updateFreeSize(_ idx: Int, newValue: Int64) {
+        self.queue.async(flags: .barrier) {
+            self.array[idx].free = newValue
         }
-        
-        return nil
     }
     
-    mutating func removeDiskByBSDName(_ name: String) {
-        if let idx = self.list.firstIndex(where: { $0.BSDName == name }) {
-            self.list.remove(at: idx)
+    func updateReadWrite(_ idx: Int, read: Int64, write: Int64) {
+        self.queue.async(flags: .barrier) {
+            self.array[idx].activity.readBytes = read
+            self.array[idx].activity.writeBytes = write
+        }
+    }
+    
+    func updateRead(_ idx: Int, newValue: Int64) {
+        self.queue.async(flags: .barrier) {
+            self.array[idx].activity.read = newValue
+        }
+    }
+    
+    func updateWrite(_ idx: Int, newValue: Int64) {
+        self.queue.async(flags: .barrier) {
+            self.array[idx].activity.write = newValue
         }
     }
 }
 
 public class Disk: Module {
     private let popupView: Popup = Popup()
-    private var activityReader: ActivityReader? = nil
     private var capacityReader: CapacityReader? = nil
+    private var activityReader: ActivityReader? = nil
     private var settingsView: Settings
     private var selectedDisk: String = ""
     
@@ -102,18 +140,22 @@ public class Disk: Module {
         guard self.available else { return }
         
         self.capacityReader = CapacityReader()
-        self.activityReader = ActivityReader(list: &self.capacityReader!.disks)
+        self.activityReader = ActivityReader()
         self.selectedDisk = Store.shared.string(key: "\(self.config.name)_disk", defaultValue: self.selectedDisk)
         
         self.capacityReader?.callbackHandler = { [unowned self] value in
-            self.capacityCallback(value)
+            if let value = value {
+                self.capacityCallback(value)
+            }
         }
         self.capacityReader?.readyCallback = { [unowned self] in
             self.readyHandler()
         }
         
         self.activityReader?.callbackHandler = { [unowned self] value in
-            self.capacityCallback(value)
+            if let value = value {
+                self.activityCallback(value)
+            }
         }
         
         self.settingsView.selectedDiskHandler = { [unowned self] value in
@@ -138,17 +180,13 @@ public class Disk: Module {
         }
     }
     
-    private func capacityCallback(_ raw: DiskList?) {
-        guard raw != nil, let value = raw else {
-            return
-        }
-        
+    private func capacityCallback(_ value: Disks) {
         DispatchQueue.main.async(execute: {
-            self.popupView.usageCallback(value)
+            self.popupView.capacityCallback(value)
         })
         self.settingsView.setList(value)
         
-        guard let d = value.getDiskByName(self.selectedDisk) ?? value.getRootDisk() else {
+        guard let d = value.first(where: { $0.mediaName == self.selectedDisk }) ?? value.first(where: { $0.root }) else {
             return
         }
         
@@ -165,6 +203,22 @@ public class Disk: Module {
             case let widget as Mini: widget.setValue(percentage)
             case let widget as BarChart: widget.setValue([percentage])
             case let widget as MemoryWidget: widget.setValue((DiskSize(free).getReadableMemory(), DiskSize(usedSpace).getReadableMemory()))
+            default: break
+            }
+        }
+    }
+    
+    private func activityCallback(_ value: Disks) {
+        DispatchQueue.main.async(execute: {
+            self.popupView.activityCallback(value)
+        })
+        
+        guard let d = value.first(where: { $0.mediaName == self.selectedDisk }) ?? value.first(where: { $0.root }) else {
+            return
+        }
+        
+        self.widgets.filter{ $0.isActive }.forEach { (w: Widget) in
+            switch w.item {
             case let widget as SpeedWidget: widget.setValue(upload: d.activity.write, download: d.activity.read)
             default: break
             }

Modules/Disk/popup.swift

@@ -27,15 +27,15 @@ internal class Popup: NSView, Popup_p {
         fatalError("init(coder:) has not been implemented")
     }
     
-    internal func usageCallback(_ value: DiskList) {
-        if self.list.count != value.list.count && self.list.count != 0 {
+    internal func capacityCallback(_ value: Disks) {
+        if self.list.count != value.count && self.list.count != 0 {
             self.subviews.forEach{ $0.removeFromSuperview() }
             self.list = [:]
         }
         
-        value.list.reversed().forEach { (drive: drive) in
+        value.reversed().forEach { (drive: drive) in
             if let disk = self.list[drive.mediaName] {
-                disk.update(free: drive.free, read: drive.activity.read, write: drive.activity.write)
+                disk.updateFree(free: drive.free)
             } else {
                 let disk = DiskView(
                     NSRect(
@@ -60,6 +60,14 @@ internal class Popup: NSView, Popup_p {
             self.sizeCallback?(self.frame.size)
         }
     }
+    
+    internal func activityCallback(_ value: Disks) {
+        value.reversed().forEach { (drive: drive) in
+            if let disk = self.list[drive.mediaName] {
+                disk.updateReadWrite(read: drive.activity.read, write: drive.activity.write)
+            }
+        }
+    }
 }
 
 internal class DiskView: NSView {
@@ -102,10 +110,14 @@ internal class DiskView: NSView {
         self.layer?.backgroundColor = isDarkMode ? NSColor(hexString: "#111111", alpha: 0.25).cgColor : NSColor(hexString: "#f5f5f5", alpha: 1).cgColor
     }
     
-    public func update(free: Int64, read: Int64, write: Int64) {
-        self.nameAndBarView.update(free: free, read: read, write: write)
+    public func updateFree(free: Int64) {
+        self.nameAndBarView.update(free: free, read: nil, write: nil)
         self.legendView.update(free: free)
     }
+    
+    public func updateReadWrite(read: Int64, write: Int64) {
+        self.nameAndBarView.update(free: nil, read: read, write: write)
+    }
 }
 
 internal class DiskNameAndBarView: NSView {
@@ -213,16 +225,20 @@ internal class DiskNameAndBarView: NSView {
         self.addSubview(view)
     }
     
-    public func update(free: Int64, read: Int64, write: Int64) {
+    public func update(free: Int64?, read: Int64?, write: Int64?) {
         if (self.window?.isVisible ?? false) || !self.ready {
-            if self.usedBarSpace != nil {
+            if let free = free, self.usedBarSpace != nil {
                 let percentage = CGFloat(self.size - free) / CGFloat(self.size)
                 let width: CGFloat = ((self.frame.width - 2) * (percentage < 0 ? 0 : percentage)) / 1
                 self.usedBarSpace?.setFrameSize(NSSize(width: width, height: self.usedBarSpace!.frame.height))
             }
             
-            self.readState?.layer?.backgroundColor = read != 0 ? NSColor.systemBlue.cgColor : NSColor.lightGray.withAlphaComponent(0.75).cgColor
-            self.writeState?.layer?.backgroundColor = write != 0 ? NSColor.systemRed.cgColor : NSColor.lightGray.withAlphaComponent(0.75).cgColor
+            if let read = read {
+                self.readState?.layer?.backgroundColor = read != 0 ? NSColor.systemBlue.cgColor : NSColor.lightGray.withAlphaComponent(0.75).cgColor
+            }
+            if let write = write {
+                self.writeState?.layer?.backgroundColor = write != 0 ? NSColor.systemRed.cgColor : NSColor.lightGray.withAlphaComponent(0.75).cgColor
+            }
             
             self.ready = true
         }

Modules/Disk/readers.swift

@@ -16,8 +16,8 @@ import IOKit
 import Darwin
 import os.log
 
-internal class CapacityReader: Reader<DiskList> {
-    internal var disks: DiskList = DiskList()
+internal class CapacityReader: Reader<Disks> {
+    internal var list: Disks = Disks()
     
     public override func read() {
         let keys: [URLResourceKey] = [.volumeNameKey]
@@ -37,134 +37,42 @@ internal class CapacityReader: Reader<DiskList> {
                         let BSDName: String = String(cString: diskName)
                         active.append(BSDName)
                         
-                        if let d: drive = self.disks.getDiskByBSDName(BSDName) {
-                            if let idx = self.disks.list.firstIndex(where: { $0.BSDName == BSDName }) {
-                                if d.removable && !removableState {
-                                    self.disks.list.remove(at: idx)
-                                    continue
-                                }
-                                
-                                if let path = self.disks.list[idx].path {
-                                    self.disks.list[idx].free = self.freeDiskSpaceInBytes(path)
-                                }
+                        if let d = self.list.first(where: { $0.BSDName == BSDName}), let idx = self.list.index(where: { $0.BSDName == BSDName}) {
+                            if d.removable && !removableState {
+                                self.list.remove(at: idx)
+                                continue
                             }
+                            
+                            if let path = d.path {
+                                self.list.updateFreeSize(idx, newValue: self.freeDiskSpaceInBytes(path))
+                            }
+                            
                             continue
                         }
                         
-                        if let d = driveDetails(disk, removableState: removableState) {
-                            self.disks.list.append(d)
-                            self.disks.list.sort{ $1.removable }
+                        if var d = driveDetails(disk, removableState: removableState) {
+                            if let path = d.path {
+                                d.free = self.freeDiskSpaceInBytes(path)
+                            }
+                            self.list.append(d)
+                            self.list.sort()
                         }
                     }
                 }
             }
         }
         
-        if active.count < self.disks.list.count {
-            let missingDisks = active.difference(from: self.disks.list.map{ $0.BSDName })
+        if active.count < self.list.count {
+            let missingDisks = active.difference(from: self.list.map{ $0.BSDName })
             
             missingDisks.forEach { (BSDName: String) in
-                self.disks.removeDiskByBSDName(BSDName)
-            }
-        }
-        
-        self.callback(self.disks)
-    }
-    
-    private func driveDetails(_ disk: DADisk, removableState: Bool) -> drive? {
-        var d: drive = drive()
-        
-        if let bsdName = DADiskGetBSDName(disk) {
-            d.BSDName = String(cString: bsdName)
-        }
-        
-        if let diskDescription = DADiskCopyDescription(disk) {
-            if let dict = diskDescription as? [String: AnyObject] {
-                if let removable = dict[kDADiskDescriptionMediaRemovableKey as String] {
-                    if removable as! Bool {
-                        if !removableState {
-                            return nil
-                        }
-                        d.removable = true
-                    }
-                }
-                
-                if let mediaName = dict[kDADiskDescriptionVolumeNameKey as String] {
-                    d.mediaName = mediaName as! String
-                    if d.mediaName == "Recovery" {
-                        return nil
-                    }
-                }
-                if d.mediaName == "" {
-                    if let mediaName = dict[kDADiskDescriptionMediaNameKey as String] {
-                        d.mediaName = mediaName as! String
-                        if d.mediaName == "Recovery" {
-                            return nil
-                        }
-                    }
-                }
-                if let mediaSize = dict[kDADiskDescriptionMediaSizeKey as String] {
-                    d.size = Int64(truncating: mediaSize as! NSNumber)
-                }
-                if let deviceModel = dict[kDADiskDescriptionDeviceModelKey as String] {
-                    d.model = (deviceModel as! String).trimmingCharacters(in: .whitespacesAndNewlines)
-                }
-                if let deviceProtocol = dict[kDADiskDescriptionDeviceProtocolKey as String] {
-                    d.connectionType = deviceProtocol as! String
-                }
-                if let volumePath = dict[kDADiskDescriptionVolumePathKey as String] {
-                    if let url = volumePath as? NSURL {
-                        d.path = url as URL
-                        
-                        if let components = url.pathComponents {
-                            d.root = components.count == 1
-                            
-                            if components.count > 1 && components[1] == "Volumes" {
-                                if let name: String = url.lastPathComponent, name != "" {
-                                    d.mediaName = name
-                                }
-                            }
-                        }
-                    }
-                }
-                if let volumeKind = dict[kDADiskDescriptionVolumeKindKey as String] {
-                    d.fileSystem = volumeKind as! String
+                if let idx = self.list.index(where: { $0.BSDName == BSDName }) {
+                    self.list.remove(at: idx)
                 }
             }
         }
         
-        if d.path == nil {
-            return nil
-        }
-        
-        if let path = d.path {
-            d.free = freeDiskSpaceInBytes(path)
-        }
-        
-        let partitionLevel = d.BSDName.filter { "0"..."9" ~= $0 }.count
-        if let parent = self.getDeviceIOParent(DADiskCopyIOMedia(disk), level: Int(partitionLevel)) {
-            d.parent = parent
-        }
-        
-        return d
-    }
-    
-    // https://opensource.apple.com/source/bless/bless-152/libbless/APFS/BLAPFSUtilities.c.auto.html
-    public func getDeviceIOParent(_ obj: io_registry_entry_t, level: Int) -> io_registry_entry_t? {
-        var parent: io_registry_entry_t = 0
-        
-        if IORegistryEntryGetParentEntry(obj, kIOServicePlane, &parent) != KERN_SUCCESS {
-            return nil
-        }
-        
-        for _ in 1...level {
-            if IORegistryEntryGetParentEntry(parent, kIOServicePlane, &parent) != KERN_SUCCESS {
-                IOObjectRelease(parent)
-                return nil
-            }
-        }
-        
-        return parent
+        self.callback(self.list)
     }
     
     private func freeDiskSpaceInBytes(_ path: URL) -> Int64 {
@@ -192,11 +100,10 @@ internal class CapacityReader: Reader<DiskList> {
     }
 }
 
-internal class ActivityReader: Reader<DiskList> {
-    internal var disks: UnsafeMutablePointer<DiskList>? = nil
+internal class ActivityReader: Reader<Disks> {
+    internal var list: Disks = Disks()
     
-    init(list: UnsafeMutablePointer<DiskList>?) {
-        self.disks = list
+    init() {
         super.init()
     }
     
@@ -205,35 +112,166 @@ internal class ActivityReader: Reader<DiskList> {
     }
     
     public override func read() {
-        guard let disks = self.disks else {
+        let keys: [URLResourceKey] = [.volumeNameKey]
+        let removableState = Store.shared.bool(key: "Disk_removable", defaultValue: false)
+        let paths = FileManager.default.mountedVolumeURLs(includingResourceValuesForKeys: keys)!
+        
+        guard let session = DASessionCreate(kCFAllocatorDefault) else {
+            os_log(.error, log: log, "cannot create a DASessionCreate()")
             return
         }
         
-        for (i, d) in disks.pointee.list.enumerated() {
-            guard let props = getIOProperties(d.parent) else {
-                return
-            }
-            
-            if let statistics = props.object(forKey: "Statistics") as? NSDictionary {
-                let readBytes = statistics.object(forKey: "Bytes (Read)") as? Int64 ?? 0
-                let writeBytes = statistics.object(forKey: "Bytes (Write)") as? Int64 ?? 0
-                
-                if disks.pointee.list[i].activity.readBytes != 0 {
-                    disks.pointee.list[i].activity.read = readBytes - disks.pointee.list[i].activity.readBytes
+        var active: [String] = []
+        for url in paths {
+            if url.pathComponents.count == 1 || (url.pathComponents.count > 1 && url.pathComponents[1] == "Volumes") {
+                if let disk = DADiskCreateFromVolumePath(kCFAllocatorDefault, session, url as CFURL) {
+                    if let diskName = DADiskGetBSDName(disk) {
+                        let BSDName: String = String(cString: diskName)
+                        active.append(BSDName)
+                        
+                        if let d = self.list.first(where: { $0.BSDName == BSDName}), let idx = self.list.index(where: { $0.BSDName == BSDName}) {
+                            if d.removable && !removableState {
+                                self.list.remove(at: idx)
+                                continue
+                            }
+                            
+                            self.driveStats(idx, d)
+                            continue
+                        }
+                        
+                        if let d = driveDetails(disk, removableState: removableState) {
+                            self.list.append(d)
+                            self.list.sort()
+                        }
+                    }
                 }
-                if disks.pointee.list[i].activity.writeBytes != 0 {
-                    disks.pointee.list[i].activity.write = writeBytes - disks.pointee.list[i].activity.writeBytes
-                }
-                
-                disks.pointee.list[i].activity.readBytes = readBytes
-                disks.pointee.list[i].activity.writeBytes = writeBytes
-                disks.pointee.list[i].activity.readOperations = statistics.object(forKey: "Operations (Read)") as? Int64 ?? 0
-                disks.pointee.list[i].activity.writeOperations = statistics.object(forKey: "Operations (Read)") as? Int64 ?? 0
-                disks.pointee.list[i].activity.readTime = statistics.object(forKey: "Total Time (Read)") as? Int64 ?? 0
-                disks.pointee.list[i].activity.writeTime = statistics.object(forKey: "Total Time (Read)") as? Int64 ?? 0
             }
         }
         
-        self.callback(disks.pointee)
+        if active.count < self.list.count {
+            let missingDisks = active.difference(from: self.list.map{ $0.BSDName })
+            
+            missingDisks.forEach { (BSDName: String) in
+                if let idx = self.list.index(where: { $0.BSDName == BSDName }) {
+                    self.list.remove(at: idx)
+                }
+            }
+        }
+        
+        self.callback(self.list)
+    }
+    
+    private func driveStats(_ idx: Int, _ d: drive) {
+        guard let props = getIOProperties(d.parent) else {
+            return
+        }
+        
+        if let statistics = props.object(forKey: "Statistics") as? NSDictionary {
+            let readBytes = statistics.object(forKey: "Bytes (Read)") as? Int64 ?? 0
+            let writeBytes = statistics.object(forKey: "Bytes (Write)") as? Int64 ?? 0
+            
+            if d.activity.readBytes != 0 {
+                self.list.updateRead(idx, newValue: readBytes - d.activity.readBytes)
+            }
+            if d.activity.writeBytes != 0 {
+                self.list.updateWrite(idx, newValue: writeBytes - d.activity.writeBytes)
+            }
+            
+            self.list.updateReadWrite(idx, read: readBytes, write: writeBytes)
+        }
+        
+        return
     }
 }
+
+private func driveDetails(_ disk: DADisk, removableState: Bool) -> drive? {
+    var d: drive = drive()
+    
+    if let bsdName = DADiskGetBSDName(disk) {
+        d.BSDName = String(cString: bsdName)
+    }
+    
+    if let diskDescription = DADiskCopyDescription(disk) {
+        if let dict = diskDescription as? [String: AnyObject] {
+            if let removable = dict[kDADiskDescriptionMediaRemovableKey as String] {
+                if removable as! Bool {
+                    if !removableState {
+                        return nil
+                    }
+                    d.removable = true
+                }
+            }
+            
+            if let mediaName = dict[kDADiskDescriptionVolumeNameKey as String] {
+                d.mediaName = mediaName as! String
+                if d.mediaName == "Recovery" {
+                    return nil
+                }
+            }
+            if d.mediaName == "" {
+                if let mediaName = dict[kDADiskDescriptionMediaNameKey as String] {
+                    d.mediaName = mediaName as! String
+                    if d.mediaName == "Recovery" {
+                        return nil
+                    }
+                }
+            }
+            if let mediaSize = dict[kDADiskDescriptionMediaSizeKey as String] {
+                d.size = Int64(truncating: mediaSize as! NSNumber)
+            }
+            if let deviceModel = dict[kDADiskDescriptionDeviceModelKey as String] {
+                d.model = (deviceModel as! String).trimmingCharacters(in: .whitespacesAndNewlines)
+            }
+            if let deviceProtocol = dict[kDADiskDescriptionDeviceProtocolKey as String] {
+                d.connectionType = deviceProtocol as! String
+            }
+            if let volumePath = dict[kDADiskDescriptionVolumePathKey as String] {
+                if let url = volumePath as? NSURL {
+                    d.path = url as URL
+                    
+                    if let components = url.pathComponents {
+                        d.root = components.count == 1
+                        
+                        if components.count > 1 && components[1] == "Volumes" {
+                            if let name: String = url.lastPathComponent, name != "" {
+                                d.mediaName = name
+                            }
+                        }
+                    }
+                }
+            }
+            if let volumeKind = dict[kDADiskDescriptionVolumeKindKey as String] {
+                d.fileSystem = volumeKind as! String
+            }
+        }
+    }
+    
+    if d.path == nil {
+        return nil
+    }
+    
+    let partitionLevel = d.BSDName.filter { "0"..."9" ~= $0 }.count
+    if let parent = getDeviceIOParent(DADiskCopyIOMedia(disk), level: Int(partitionLevel)) {
+        d.parent = parent
+    }
+    
+    return d
+}
+
+// https://opensource.apple.com/source/bless/bless-152/libbless/APFS/BLAPFSUtilities.c.auto.html
+public func getDeviceIOParent(_ obj: io_registry_entry_t, level: Int) -> io_registry_entry_t? {
+    var parent: io_registry_entry_t = 0
+    
+    if IORegistryEntryGetParentEntry(obj, kIOServicePlane, &parent) != KERN_SUCCESS {
+        return nil
+    }
+    
+    for _ in 1...level {
+        if IORegistryEntryGetParentEntry(parent, kIOServicePlane, &parent) != KERN_SUCCESS {
+            IOObjectRelease(parent)
+            return nil
+        }
+    }
+    
+    return parent
+}

Modules/Disk/settings.swift

@@ -96,8 +96,8 @@ internal class Settings: NSView, Settings_v {
         self.addSubview(view)
     }
     
-    internal func setList(_ list: DiskList) {
-        let disks = list.list.map{ $0.mediaName }
+    internal func setList(_ list: Disks) {
+        let disks = list.map{ $0.mediaName }
         DispatchQueue.main.async(execute: {
             if self.button?.itemTitles.count != disks.count {
                 self.button?.removeAllItems()