foursquare/FSNetworking

BUG Running FSNetworking in background threads

tomandersen opened this issue · 4 comments

I don't know if its supported, but it seems to work. Mostly.

In FSNConnection, there is the mutableset of current connections.

mutableConnections

It is a NSMutableSet and is a global, but accessed by eac h object with no @synchronize() {} block.

So the code need to change in two places -

In cleanup

NSMutableSet *requests = [self.class mutableConnections];
@synchronized(requests) {
    [requests removeObject:self];
}

And in start
NSMutableSet *connections = [self.class mutableConnections];
@synchronized(connections) {
[connections addObject:self];
}

I got a crash (as would make sense) when using about 3 of these on threads on 10.8.4 Mac, with XCode 5. With Zombies and what not on, it pointed right at the cleanup line.

--Tom

gwk commented

Thank you for the report. I will attempt to address this (and any other outstanding community issues) after 9/22. Anyone else is welcome to create a pull request before then; I'm no longer a maintainer but would like to help keep the project alive.

George

On Sep 11, 2013, at 9:41 AM, tomandersen notifications@github.com wrote:

I don't know if its supported, but it seems to work. Mostly.

In FSNConnection, there is the mutableset of current connections.

mutableConnections

It is a NSMutableSet and is a global, but accessed by eac h object with no @synchronize() {} block.

So the code need to change in two places -

In cleanup

NSMutableSet *requests = [self.class mutableConnections];
@synchronized(requests) {
[requests removeObject:self];
}
And in start
NSMutableSet *connections = [self.class mutableConnections];
@synchronized(connections) {
[connections addObject:self];
}

I got a crash (as would make sense) when using about 3 of these on threads on 10.8.4 Mac, with XCode 5. With Zombies and what not on, it pointed right at the cleanup line.

--Tom


Reply to this email directly or view it on GitHub.

gwk commented

I looked at this some more, and I am not sure that those changes alone are sufficient to make FSNConnection truly multithread safe (as usual, it's hard to be sure when it comes to multithreading). May I ask why you cannot just call start from the main thread? All of your completion blocks will get called on the main thread anyway, so there does not seem to be a performance benefit.

The call prepares data, which takes 0,2 to 2 seconds, most of which is waiting on disk or other system calls. So I run 8 at once on threads.(operations). When the data is ready, it's sent. Then the reply is parsed. The main thread bit is very small. If I pop to the main thread for the send, the calls can pile up or get stuck behind user interface code, which is yucky.

With that fix my luck has been good. I can run millions of operations in a process that runs for a month or more.

So I agree - I'm not sure it's fully thread safe either, but works for me.

One way to expose bugs is to set up some massive number of fsn connections running at the same time. Not done yet.

Tom

Sent from my iPhone

On 2013-10-02, at 5:19 PM, George King notifications@github.com wrote:

I looked at this some more, and I am not sure that those changes alone are sufficient to make FSNConnection truly multithread safe (as usual, it's hard to be sure when it comes to multithreading). May I ask why you cannot just call start from the main thread? All of your completion blocks will get called on the main thread anyway, so there does not seem to be a performance benefit.


Reply to this email directly or view it on GitHub.

gwk commented

OK. An intermediate solution might be to create the FSNConnection object on your background thread (clearly thread safe because it is a new object) and then call:
[connection performSelectorOnMainThread:@selector(start) withObject:nil waitUntilDone:NO];
Then (if I understand you correctly) the long data preparation is multithreaded but FSNConnection need not be.