Skip to content

Commit

Permalink
Update atomics APIs to modern replacements
Browse files Browse the repository at this point in the history
- OSSpinLock replaced with os_unfair_lock (possibly some of these uses could avoid os_unfair_lock)
- OSAtomic replaced with atomic_ functions
  • Loading branch information
bangnoise committed Jan 8, 2023
1 parent 1aa9df0 commit fc4f4a2
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 123 deletions.
31 changes: 15 additions & 16 deletions SyphonClientBase.m
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,13 @@
#import "SyphonServerDirectory.h"
#import "SyphonClientConnectionManager.h"
#import "SyphonPrivate.h"
#import <os/lock.h>

// TODO: name?
static void *SyphonClientServersContext = &SyphonClientServersContext;

@implementation SyphonClientBase {
// Once our minimum version reaches 10.12, replace
// this with os_unfair_lock
OSSpinLock _lock;
os_unfair_lock _lock;
NSUInteger _lastFrameID;
SyphonClientConnectionManager *_connectionManager;
NSDictionary<NSString *, id> *_serverDescription;
Expand Down Expand Up @@ -68,7 +67,7 @@ - (instancetype)initWithServerDescription:(NSDictionary<NSString *, id> *)descri
self = [super init];
if (self)
{
_lock = OS_SPINLOCK_INIT;
_lock = OS_UNFAIR_LOCK_INIT;

_connectionManager = [[SyphonClientConnectionManager alloc] initWithServerDescription:description];

Expand Down Expand Up @@ -96,9 +95,9 @@ - (instancetype)initWithServerDescription:(NSDictionary<NSString *, id> *)descri

- (BOOL)isValid
{
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
BOOL result = _connectionManager.isValid;
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
return result;
}

Expand All @@ -116,14 +115,14 @@ - (void)stop

- (void)stopBase
{
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
if (_connectionManager)
{
[_connectionManager removeInfoClient:(id <SyphonInfoReceiving>)self
isFrameClient:_handler != nil ? YES : NO];
_connectionManager = nil;
}
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
}

- (void)receiveNewFrame
Expand All @@ -142,17 +141,17 @@ - (void)invalidateFrame
- (BOOL)hasNewFrame
{
BOOL result;
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
result = _lastFrameID != _connectionManager.frameID;
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
return result;
}

- (NSDictionary *)serverDescription
{
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
NSDictionary *description = _serverDescription;
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
return description;
}

Expand All @@ -172,9 +171,9 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
{
[self willChangeValueForKey:@"serverDescription"];
NSDictionary *copied = [description copy];
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
_serverDescription = copied;
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
[self didChangeValueForKey:@"serverDescription"];
}
}
Expand All @@ -189,10 +188,10 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
- (IOSurfaceRef)newSurface
{
IOSurfaceRef surface;
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
_lastFrameID = [_connectionManager frameID];
surface = [_connectionManager newSurface];
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
return surface;
}

Expand Down
59 changes: 30 additions & 29 deletions SyphonClientConnectionManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -32,42 +32,43 @@
#import "SyphonPrivate.h"
#import "SyphonMessaging.h"
#import <IOSurface/IOSurface.h>
#import <libkern/OSAtomic.h>
#import <os/lock.h>
#import <stdatomic.h>

#pragma mark Shared Instances

static OSSpinLock _lookupTableLock = OS_SPINLOCK_INIT;
static os_unfair_lock _lookupTableLock = OS_UNFAIR_LOCK_INIT;
static NSMapTable *_lookupTable;

static id SyphonClientPrivateCopyInstance(NSString *uuid)
{
id result = nil;
OSSpinLockLock(&_lookupTableLock);
os_unfair_lock_lock(&_lookupTableLock);
if (uuid) result = [_lookupTable objectForKey:uuid];
OSSpinLockUnlock(&_lookupTableLock);
os_unfair_lock_unlock(&_lookupTableLock);
return result;
}

static void SyphonClientPrivateInsertInstance(id instance, NSString *uuid)
{
OSSpinLockLock(&_lookupTableLock);
os_unfair_lock_lock(&_lookupTableLock);
if (uuid)
{
if (!_lookupTable) _lookupTable = [[NSMapTable alloc] initWithKeyOptions:NSMapTableStrongMemory valueOptions:NSMapTableWeakMemory capacity:1];
[_lookupTable setObject:instance forKey:uuid];
}
OSSpinLockUnlock(&_lookupTableLock);
os_unfair_lock_unlock(&_lookupTableLock);
}

static void SyphonClientPrivateRemoveInstance(id instance, NSString *uuid)
{
OSSpinLockLock(&_lookupTableLock);
os_unfair_lock_lock(&_lookupTableLock);
if (uuid) [_lookupTable removeObjectForKey:uuid];
if ([_lookupTable count] == 0)
{
_lookupTable = nil;
}
OSSpinLockUnlock(&_lookupTableLock);
os_unfair_lock_unlock(&_lookupTableLock);
}

@interface SyphonClientConnectionManager (Private)
Expand All @@ -88,11 +89,11 @@ @implementation SyphonClientConnectionManager
NSString *_serverUUID;
BOOL _serverActive;
SyphonMessageReceiver *_connection;
int32_t _handlerCount;
atomic_int _handlerCount;
NSHashTable *_infoClients;
NSHashTable *_frameClients;
dispatch_queue_t _frameQueue;
OSSpinLock _lock;
os_unfair_lock _lock;
}

- (id)initWithServerDescription:(NSDictionary *)description
Expand Down Expand Up @@ -121,7 +122,7 @@ - (id)initWithServerDescription:(NSDictionary *)description
return nil;
}

_lock = OS_SPINLOCK_INIT;
_lock = OS_UNFAIR_LOCK_INIT;
_myUUID = SyphonCreateUUIDString();
_serverActive = YES; // Until we know better - SyphonClient has API behaviour depending on this

Expand All @@ -140,20 +141,20 @@ - (void)endConnectionHavingLock:(BOOL)hasLock
SYPHONLOG(@"Ending connection");
SyphonMessageReceiver *connection;
// we copy and clear ivars inside the lock, release them outside it
if (!hasLock) OSSpinLockLock(&_lock);
if (!hasLock) os_unfair_lock_lock(&_lock);
connection = _connection;
_connection = nil;
[self invalidateFramesHavingLock];
if (!hasLock) OSSpinLockUnlock(&_lock);
if (!hasLock) os_unfair_lock_unlock(&_lock);
[connection invalidate];
}

- (void)invalidateServerNotHavingLock
{
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
_serverActive = NO;
[self endConnectionHavingLock:YES];
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
}

- (void)invalidateFramesHavingLock
Expand All @@ -171,15 +172,15 @@ - (void)invalidateFramesHavingLock
- (BOOL)isValid
{
BOOL result;
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
result = _serverActive;
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
return result;
}

- (void)addInfoClient:(id <SyphonInfoReceiving>)client isFrameClient:(BOOL)isFrameClient
{
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
if (_infoClients == nil)
{
_infoClients = [NSHashTable weakObjectsHashTable];
Expand Down Expand Up @@ -224,7 +225,7 @@ - (void)addInfoClient:(id <SyphonInfoReceiving>)client isFrameClient:(BOOL)isFra
_frameQueue = dispatch_queue_create([_myUUID cStringUsingEncoding:NSUTF8StringEncoding], 0);
_frameClients = [NSHashTable weakObjectsHashTable];
}
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
if (isFrameClient)
{
// only access _frameClients within the queue
Expand All @@ -249,7 +250,7 @@ - (void)addInfoClient:(id <SyphonInfoReceiving>)client isFrameClient:(BOOL)isFra
SYPHONLOG(@"Registering for info updates");
[sender send:_myUUID ofType:SyphonMessageTypeAddClientForInfo];
}
if (isFrameClient && OSAtomicIncrement32(&_handlerCount) == 1)
if (isFrameClient && atomic_fetch_add(&_handlerCount, 1) == 0)
{
SYPHONLOG(@"Registering for frame updates");
[sender send:_myUUID ofType:SyphonMessageTypeAddClientForFrames];
Expand All @@ -265,22 +266,22 @@ - (void)removeInfoClient:(id <SyphonInfoReceiving>)client isFrameClient:(BOOL)is
[_frameClients removeObject:client];
});
}
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
[_infoClients removeObject:client];
BOOL shouldSendRemove = _infoClients.count == 0 ? YES : NO;
if (shouldSendRemove)
{
[self endConnectionHavingLock:YES];
}
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
if (_serverActive && (shouldSendRemove || isFrameClient))
{
// Remove ourself from the server
SyphonMessageSender *sender = [[SyphonMessageSender alloc] initForName:_serverUUID
protocol:SyphonMessagingProtocolCFMessage
invalidationHandler:nil];

if (isFrameClient && OSAtomicDecrement32(&_handlerCount) == 0)
if (isFrameClient && atomic_fetch_sub(&_handlerCount, 1) == 1)
{
SYPHONLOG(@"De-registering for frame updates");
[sender send:_myUUID ofType:SyphonMessageTypeRemoveClientForFrames];
Expand Down Expand Up @@ -321,27 +322,27 @@ - (IOSurfaceRef)surfaceHavingLock

- (void)setSurfaceID:(IOSurfaceID)surfaceID
{
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
_surfaceID = surfaceID;
_frameID++; // new surface means a new frame
[self invalidateFramesHavingLock];
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
}

- (IOSurfaceRef)newSurface
{
IOSurfaceRef surface;
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
surface = [self surfaceHavingLock];
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
if (surface) CFRetain(surface);
return surface;
}

- (NSUInteger)frameID
{
NSUInteger result;
OSSpinLockLock(&_lock);
os_unfair_lock_lock(&_lock);
IOSurfaceRef surface = [self surfaceHavingLock];
if (surface)
{
Expand All @@ -353,7 +354,7 @@ - (NSUInteger)frameID
}
}
result = _frameID;
OSSpinLockUnlock(&_lock);
os_unfair_lock_unlock(&_lock);
return result;
}

Expand Down
Loading

0 comments on commit fc4f4a2

Please sign in to comment.