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

Timeouts leave threads hanging #30

Closed
massenz opened this issue Feb 22, 2016 · 4 comments
Closed

Timeouts leave threads hanging #30

massenz opened this issue Feb 22, 2016 · 4 comments

Comments

@massenz
Copy link

massenz commented Feb 22, 2016

I believe I've come across an issue when running requests in many threads (one thread per request) and some (but not all) of them time out.

The timeouts seem to be "honored" but the thread that initiated the request is left waiting for a semaphore: doing some debugging in Xcode's thread view, I noticed all the threads were stuck on this line:

dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER)

when I changed it to something like:

                        let NSEC: Int64 = {
                            var base: Int64 = 1
                            for _ in 1...9 {
                                base *= 10
                            }
                            return base
                        }()

                        let tint: Int64 = Int64(timeout ?? 10.0)
                        let how_long = dispatch_time(DISPATCH_TIME_NOW, tint * NSEC)

                        dispatch_semaphore_wait(semaphore, how_long)

my code now runs without problem, I can see threads being created then destroyed, and it's now been running for > 1 hour without any issue (it used to just crash and die with a hundred or so of hung threads in a couple of minutes).

If you want, I'd be happy to polish the fix and push a PR?

@dduan
Copy link
Owner

dduan commented Feb 22, 2016

Hi @massenz, thanks for reporting this. I think we need to look into it more carefully. I don't know how you manage your threads. But in general, I would use a separate Just instance per thread:

// threaded branch
let just = JustOf<HTTP>()
// do work with this instance

@massenz
Copy link
Author

massenz commented Feb 22, 2016

My code doesn't actually create/manage threads: we simply use the dispatch_async(backgroundQueue, { // block of code } so the threads are managed by iOS.

I think in total it must have created approx 1,000 threads during the last run, but only a handful were active at any time, after the fix.

I completely agree that the code pasted above was as "quick and dirty" as it gets :) and needs more careful review: in particular, I am not sure where that Semaphore gets set/reset, but I suspect that when the request times out, it just gets "abandoned" without ever resetting the state.

Since I last posted the earlier message, I've run my code for > 1 hour, and had 200 "rounds" (that would be a few 1,000's requests) and the app is still happily running; also Memory consumption (which was ballooning) was extremely low all the time.

BTW (and I realize I should have said this earlier :) ) I totally love Just - I am a beginner in Swift and it made my life so much easier! Thanks!

@massenz
Copy link
Author

massenz commented Feb 22, 2016

Forgot to say - yes, we have an instance of Just per thread.

@dduan dduan closed this as completed in 9370259 Feb 23, 2016
dduan added a commit that referenced this issue Feb 23, 2016
[Fixes #30] Ensure threads are not left hanging, if the request times out
@dduan
Copy link
Owner

dduan commented Feb 23, 2016

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

No branches or pull requests

2 participants