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

queue based stream #25

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

queue based stream #25

wants to merge 8 commits into from

Conversation

daltoniam
Copy link
Collaborator

This is to address the concerns in #15 and #18. I believe it might fix the issues in #13. This will supersede the feature/thread #16 PR.

@@ -195,10 +195,11 @@ - (void)createHTTPRequest
(__bridge CFStringRef)headerWSHostName,
(__bridge CFStringRef)[NSString stringWithFormat:@"%@:%@",self.url.host,port]);

for(NSString *key in self.headers) {
NSDictionary *heads = [self.headers copy];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this method was moved unto the client thread, the copy is a bit pedantic.

I had suggested adding copy to the addHeader: method itself, which would make self.headers immutable and totally safe under any situation.

@mythodeia
Copy link

@adamkaplan is any of this still valuable that can be merged in master?

@adamkaplan
Copy link
Contributor

Yeah..... lot of great stuff in there actually. But it still fails the Autobahn high-performance tests. I haven't had time to fix it and probably won't for quite a while.

It hurts to leave this incomplete for so long 😢

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

Successfully merging this pull request may close these issues.

3 participants