Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Messaging happens on main thread #20

Closed
bangnoise opened this issue Mar 31, 2015 · 3 comments
Closed

Messaging happens on main thread #20

bangnoise opened this issue Mar 31, 2015 · 3 comments
Labels

Comments

@bangnoise
Copy link
Member

@bangnoise bangnoise commented Mar 31, 2015

From Google Code:
May be made moot if we become sandboxable - see #10.

Reported by bangnoise, Nov 3, 2010

Currently SyphonClient invokes any new-frame handler on the main thread. This means that drawing may happen on the main thread, and that new-frame handlers can never be invoked in parallel for different clients.
This is because currently SyphonCFMessageReceiver installs its CFRunLoopSource on the main thread.
We could use SyphonDispatch to invoke new-frame handlers on their own threads. Currently SyphonDispatch does no exception handling. We should probably address that if we start using it to call user code.
Additionally we could consider installing SyphonCFMessageReceiver's CFRunLoopSource on a dedicated thread.

Apr 16, 2011 bangnoise
(No comment was entered for this change.)
Labels: -Priority-Medium -Milestone-Release1.0 Priority-High Milestone-PublicBeta2

Jun 1, 2011 bangnoise
Fixed in r33
Status: Fixed

Jun 13, 2011 bangnoise
Either there's a subtle bug in SyphonMessaging somewhere or a bug in CFMessagePortSetDispatchQueue() (under 10.6.7 but not the next major revision) causing SyphonMessageReceivers to never receive messages sent to them in some circumstances. This needs some more investigation to pinpoint the source of the bug (in the OS or Syphon), and if in the OS avoid using CFMessagePortSetDispatchQueue().
Status: Accepted

Jun 13, 2011 bangnoise
I've undone the change in public-beta-2 so its behaviour is as public beta 1 (messages are received on the main thread).
Labels: -Milestone-PublicBeta2 Milestone-PublicBeta3

Jun 13, 2011 vade@vade.info
So 10.6.8 behavior in CFMessagePortSetDispatchQueue() works as expected and the non main thread messaging works fine? Or do you mean 10.7 ?

Jun 17, 2011 bangnoise
I don't mean 10.6.8. Shh! ;)

Feb 4, 2012 bangnoise
r60 reverts r33 pending a real fix for this

Feb 4, 2012 vade@vade.info
Is this something we could change between 10.7 and 10.6.8, doing a runtime check? 10.7 seems to work with the above (now reverted fix), but 10.6.8 does not - it eventually looses handle on messaging. Perhaps 10.6.8 clients can have the main thread behavior, but 10.7 clients can use our own private queue? I'm sure you want a cleaner fix, but this might be a way to settle it and move on, for now?

Feb 4, 2012 bangnoise
I think original issue (moving messaging off main thread) is important enough that solution should work for all supported OS versions.

@mrRay
Copy link

@mrRay mrRay commented Dec 5, 2018

howdy-

i've got a simple patch that i think may be relevant to this enhancement- it adds the ability to create a SyphonClient instance that installs the message receiver on the runloop of the current thread ("current" when the SyphonClient was created).

  • it adds this ability via a new init method for SyphonClient, so existing projects wouldn't have to change anything and wouldn't experience any change in behavior:
- (id)initWithServerDescription:(NSDictionary *)description context:(CGLContextObj)context options:(NSDictionary *)options preferCurrentThread:(BOOL)pct newFrameHandler:(void (^)(SYPHON_CLIENT_UNIQUE_CLASS_NAME *client))handler;
  • it doesn't use GCD- instead, it installs the message receiver on the current thread's runloop, so theoretically the bug described in this thread wouldn't be an issue...

we've been using this in a few projects for a few months, and haven't noticed any problems (yet). should i open a pull request for this?

cheers
: : ray

@bangnoise
Copy link
Member Author

@bangnoise bangnoise commented Dec 7, 2018

Hey hey - thanks @mrRay - I'd rather not surface a dependency on run loops up to the API.

Reading over this and as we no longer support 10.6 it sounds like my original fix might work.

Would it be any use to you if SyphonClient added an init method which accepted a queue for the handler?

@mrRay
Copy link

@mrRay mrRay commented Dec 7, 2018

that sounds like a very nice addition!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants