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

UIScheduler not working as expected #2635

Closed
kommen opened this Issue Dec 28, 2015 · 13 comments

Comments

Projects
None yet
6 participants
@kommen

kommen commented Dec 28, 2015

With RAC4 rc1 .observeOn(UIScheduler()) doesn't work as expected for me (I didn't test older versions though):

    let currentRoute = MutableProperty<MKRoute?>(nil)

    func observe() {
        self.currentRoute.producer.observeOn(UIScheduler()).startWithNext { route in
// Works:
//          dispatch_async(dispatch_get_main_queue(), {
//              if let route = route {
//                  self.mapView.addOverlay(route.polyline, level: .AboveRoads)
//              }
//          })

// Crashes in -[VKRasterOverlayTileSource init] with EXC_BAD_ACCESS:
            if let route = route {
                self.mapView.addOverlay(route.polyline, level: .AboveRoads)
            }
        }
     }

currentRoute is set from the callback in MKDirections calculateDirectionsWithCompletionHandler. When I wrap the addOverlay call into a dispatch_async(dispatch_get_main_queue(), as shown in the commented version, it works as expected and doesn't crash.

Any hints on what I'm doing wrong?

@kommen

This comment has been minimized.

Show comment
Hide comment
@kommen

kommen Dec 28, 2015

I've created a minimal sample project demonstrating my issue: http://www.komendera.com/bugs/rac/UISchedulerTest.zip

kommen commented Dec 28, 2015

I've created a minimal sample project demonstrating my issue: http://www.komendera.com/bugs/rac/UISchedulerTest.zip

@kommen

This comment has been minimized.

Show comment
Hide comment
@kommen

kommen Dec 28, 2015

The problem seems to be that MKMapView wants to have its addOverlay called on the main queue, not only the main thread, so the UIScheduler's NSThread.isMainThread() check to see if it can skip scheduling is not sufficient here.

I found this tweet, which might be related?:
https://twitter.com/jspahrsummers/status/669198976835592192

kommen commented Dec 28, 2015

The problem seems to be that MKMapView wants to have its addOverlay called on the main queue, not only the main thread, so the UIScheduler's NSThread.isMainThread() check to see if it can skip scheduling is not sufficient here.

I found this tweet, which might be related?:
https://twitter.com/jspahrsummers/status/669198976835592192

@NachoSoto

This comment has been minimized.

Show comment
Hide comment
@NachoSoto

NachoSoto Dec 28, 2015

Member

Main queue and main thread are the same thing. The optimization that you linked to regarding scheduling is an implementation detail, but regardless, both will be invoked on the same thread.

What's the crash exactly? The bad access seems to indicate that something else is at play here. I'm AFK right now but I can test the sample project when I get home :)

Member

NachoSoto commented Dec 28, 2015

Main queue and main thread are the same thing. The optimization that you linked to regarding scheduling is an implementation detail, but regardless, both will be invoked on the same thread.

What's the crash exactly? The bad access seems to indicate that something else is at play here. I'm AFK right now but I can test the sample project when I get home :)

@kommen

This comment has been minimized.

Show comment
Hide comment
@kommen

kommen Dec 28, 2015

Main queue and main thread are the same thing.

I thought so too.

Here's a simple Playground demonstrating the issue without ReactiveCocoa:
https://gist.github.com/kommen/524ca1ce72df1f5d0b96

The crash log from the Playground looks like this (the framework stack trace is the same for the crashes with RAC):

28/12/15 20:55:23,602 com.apple.dt.Xcode.Playground[81153]: IDEPlaygroundExecutionSessionThread(pid=81534) IDEPlaygroundExecution: * thread #1: tid = 0xc8ce3b, 0x00000001078d0906 libdispatch.dylib`dispatch_retain, queue = 'myqueue', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001078d0906 libdispatch.dylib`dispatch_retain
    frame #1: 0x0000000112e88353 VectorKit`-[VKRasterOverlayTileSource init] + 163
    frame #2: 0x0000000112ab0451 VectorKit`-[VKMapModel _rasterOverlayTileSourceForLevel:] + 289
    frame #3: 0x0000000112ab0bd4 VectorKit`-[VKMapModel addRasterOverlay:] + 52
    frame #4: 0x000000011262432f MapKit`-[MKOverlayContainerView _insertDrawable:forOverlay:atIndex:level:] + 1365
    frame #5: 0x00000001126221a5 MapKit`-[MKOverlayContainerView _configureAndAddDrawable:forOverlay:level:] + 401
    frame #6: 0x000000011262238e MapKit`-[MKOverlayContainerView _considerAddingDrawable:inAddRect:level:] + 461
    frame #7: 0x0000000112622b6d MapKit`-[MKOverlayContainerView addOverlay:level:] + 198
    frame #8: 0x0000000112615c94 MapKit`-[MKMapView(OverlaysAPI) addOverlay:level:] + 85
    frame #9: 0x00000001142b3304 $__lldb_expr9`__lldb_expr_9.(closure #1) + 260 at playground9.swift:38
    frame #10: 0x00000001142b34de $__lldb_expr9`reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_unowned @convention(block) () -> (@unowned ()) + 46 at <EXPR>:0
    frame #11: 0x00000001078e24a7 libdispatch.dylib`_dispatch_client_callout + 8
    frame #12: 0x00000001078caf34 libdispatch.dylib`_dispatch_barrier_sync_f_invoke + 99
    frame #13: 0x00000001142afc5f $__lldb_expr9`main + 7263 at playground9.swift:46
    frame #14: 0x0000000104133840 MapKitPlayground
    frame #15: 0x0000000104136711 MapKitPlayground`reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_owned (@in ()) -> (@out ()) + 17
    frame #16: 0x00000001041361a1 MapKitPlayground`partial apply forwarder for reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_owned (@in ()) -> (@out ()) + 81
    frame #17: 0x0000000104136740 MapKitPlayground`reabstraction thunk helper from @callee_owned (@in ()) -> (@out ()) to @callee_owned () -> (@unowned ()) + 32
    frame #18: 0x0000000104136777 MapKitPlayground`reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_unowned @convention(block) () -> (@unowned ()) + 39
    frame #19: 0x0000000104711a1c CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
    frame #20: 0x00000001047076a5 CoreFoundation`__CFRunLoopDoBlocks + 341
    frame #21: 0x0000000104706e02 CoreFoundation`__CFRunLoopRun + 850
    frame #22: 0x0000000104706828 CoreFoundation`CFRunLoopRunSpecific + 488
    frame #23: 0x000000010b1e7ad2 GraphicsServices`GSEventRunModal + 161
    frame #24: 0x0000000104b92610 UIKit`UIApplicationMain + 171
    frame #25: 0x0000000104133d8a MapKitPlayground`main + 1354
    frame #26: 0x000000010791192d libdyld.dylib`start + 1
    frame #27: 0x000000010791192d libdyld.dylib`start + 1

kommen commented Dec 28, 2015

Main queue and main thread are the same thing.

I thought so too.

Here's a simple Playground demonstrating the issue without ReactiveCocoa:
https://gist.github.com/kommen/524ca1ce72df1f5d0b96

The crash log from the Playground looks like this (the framework stack trace is the same for the crashes with RAC):

28/12/15 20:55:23,602 com.apple.dt.Xcode.Playground[81153]: IDEPlaygroundExecutionSessionThread(pid=81534) IDEPlaygroundExecution: * thread #1: tid = 0xc8ce3b, 0x00000001078d0906 libdispatch.dylib`dispatch_retain, queue = 'myqueue', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001078d0906 libdispatch.dylib`dispatch_retain
    frame #1: 0x0000000112e88353 VectorKit`-[VKRasterOverlayTileSource init] + 163
    frame #2: 0x0000000112ab0451 VectorKit`-[VKMapModel _rasterOverlayTileSourceForLevel:] + 289
    frame #3: 0x0000000112ab0bd4 VectorKit`-[VKMapModel addRasterOverlay:] + 52
    frame #4: 0x000000011262432f MapKit`-[MKOverlayContainerView _insertDrawable:forOverlay:atIndex:level:] + 1365
    frame #5: 0x00000001126221a5 MapKit`-[MKOverlayContainerView _configureAndAddDrawable:forOverlay:level:] + 401
    frame #6: 0x000000011262238e MapKit`-[MKOverlayContainerView _considerAddingDrawable:inAddRect:level:] + 461
    frame #7: 0x0000000112622b6d MapKit`-[MKOverlayContainerView addOverlay:level:] + 198
    frame #8: 0x0000000112615c94 MapKit`-[MKMapView(OverlaysAPI) addOverlay:level:] + 85
    frame #9: 0x00000001142b3304 $__lldb_expr9`__lldb_expr_9.(closure #1) + 260 at playground9.swift:38
    frame #10: 0x00000001142b34de $__lldb_expr9`reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_unowned @convention(block) () -> (@unowned ()) + 46 at <EXPR>:0
    frame #11: 0x00000001078e24a7 libdispatch.dylib`_dispatch_client_callout + 8
    frame #12: 0x00000001078caf34 libdispatch.dylib`_dispatch_barrier_sync_f_invoke + 99
    frame #13: 0x00000001142afc5f $__lldb_expr9`main + 7263 at playground9.swift:46
    frame #14: 0x0000000104133840 MapKitPlayground
    frame #15: 0x0000000104136711 MapKitPlayground`reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_owned (@in ()) -> (@out ()) + 17
    frame #16: 0x00000001041361a1 MapKitPlayground`partial apply forwarder for reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_owned (@in ()) -> (@out ()) + 81
    frame #17: 0x0000000104136740 MapKitPlayground`reabstraction thunk helper from @callee_owned (@in ()) -> (@out ()) to @callee_owned () -> (@unowned ()) + 32
    frame #18: 0x0000000104136777 MapKitPlayground`reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_unowned @convention(block) () -> (@unowned ()) + 39
    frame #19: 0x0000000104711a1c CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
    frame #20: 0x00000001047076a5 CoreFoundation`__CFRunLoopDoBlocks + 341
    frame #21: 0x0000000104706e02 CoreFoundation`__CFRunLoopRun + 850
    frame #22: 0x0000000104706828 CoreFoundation`CFRunLoopRunSpecific + 488
    frame #23: 0x000000010b1e7ad2 GraphicsServices`GSEventRunModal + 161
    frame #24: 0x0000000104b92610 UIKit`UIApplicationMain + 171
    frame #25: 0x0000000104133d8a MapKitPlayground`main + 1354
    frame #26: 0x000000010791192d libdyld.dylib`start + 1
    frame #27: 0x000000010791192d libdyld.dylib`start + 1

@laptobbe

This comment has been minimized.

Show comment
Hide comment
@laptobbe

laptobbe Dec 30, 2015

I ran into the same problem, I was trying to scroll a collection view programmatically and the scroll was not happening. I tried dispatching async to main and that fixed the problem for me. I created another version of ate UISchedular that always dispatches async to main. This probably has other implications that I am unaware of but it solved the problem for me right now.

public final class AsyncUIScheduler: SchedulerType {

    public init() {}

    public func schedule(action: () -> ()) -> Disposable? {
        let disposable = SimpleDisposable()
        let actionWithDisposable: () -> () = {
            if !disposable.disposed {
                action()
            }
        }

        dispatch_async(dispatch_get_main_queue(), actionWithDisposable)

        return disposable
    }
}

I ran into the same problem, I was trying to scroll a collection view programmatically and the scroll was not happening. I tried dispatching async to main and that fixed the problem for me. I created another version of ate UISchedular that always dispatches async to main. This probably has other implications that I am unaware of but it solved the problem for me right now.

public final class AsyncUIScheduler: SchedulerType {

    public init() {}

    public func schedule(action: () -> ()) -> Disposable? {
        let disposable = SimpleDisposable()
        let actionWithDisposable: () -> () = {
            if !disposable.disposed {
                action()
            }
        }

        dispatch_async(dispatch_get_main_queue(), actionWithDisposable)

        return disposable
    }
}
@ikesyo

This comment has been minimized.

Show comment
Hide comment
@ikesyo

ikesyo Dec 30, 2015

Member

I created another version of ate UISchedular that always dispatches async to main.

That is QueueScheduler.mainQueueScheduler.

Member

ikesyo commented Dec 30, 2015

I created another version of ate UISchedular that always dispatches async to main.

That is QueueScheduler.mainQueueScheduler.

@kommen

This comment has been minimized.

Show comment
Hide comment
@kommen

kommen Dec 30, 2015

Yes, QueueScheduler.mainQueueScheduler is what I'm using now instead of UIScheduler, which works.

I suppose there's no way of knowing beforehand weither UIScheduler is safe or if one should use QueueScheduler.mainQueueScheduler?

kommen commented Dec 30, 2015

Yes, QueueScheduler.mainQueueScheduler is what I'm using now instead of UIScheduler, which works.

I suppose there's no way of knowing beforehand weither UIScheduler is safe or if one should use QueueScheduler.mainQueueScheduler?

@laptobbe

This comment has been minimized.

Show comment
Hide comment
@laptobbe

laptobbe Dec 30, 2015

@ikesyo Thanks for the tip, will use that instead 👍🏼

@ikesyo Thanks for the tip, will use that instead 👍🏼

@kommen

This comment has been minimized.

Show comment
Hide comment
@kommen

kommen Jan 9, 2016

@NachoSoto I contacted Apple DTS, they confirmed it is a bug in MapKit. But they also cautioned that the pattern of dispatch_sync and the isMainThread will likely run into more issues like this and the one reported by @laptobbe above.

They also explicitly stated that main queue and the main thread are not the same thing, but are have subtle differences, as shown in my demo app.

kommen commented Jan 9, 2016

@NachoSoto I contacted Apple DTS, they confirmed it is a bug in MapKit. But they also cautioned that the pattern of dispatch_sync and the isMainThread will likely run into more issues like this and the one reported by @laptobbe above.

They also explicitly stated that main queue and the main thread are not the same thing, but are have subtle differences, as shown in my demo app.

@kommen

This comment has been minimized.

Show comment
Hide comment
@kommen

kommen Jan 9, 2016

For what its worth, I dug into what's the issue in my example:

[VKRasterOverlayTileSource init] calls +[VKDispatch defaultDispatch] on the current dispatch queue, which in my example is the org.reactivecocoa.ReactiveCocoa.SignalProducer.buffer serial queue.
There dispatch_get_specific("com.apple.vectorkit.dispatch.homequeue"); is called, which returns null, because the key wasn't set on that queue, but only on the main queue. This eventually leads to a call to dispatch_retain passing null in -[VKRasterOverlayTileSource init] which crashes.

kommen commented Jan 9, 2016

For what its worth, I dug into what's the issue in my example:

[VKRasterOverlayTileSource init] calls +[VKDispatch defaultDispatch] on the current dispatch queue, which in my example is the org.reactivecocoa.ReactiveCocoa.SignalProducer.buffer serial queue.
There dispatch_get_specific("com.apple.vectorkit.dispatch.homequeue"); is called, which returns null, because the key wasn't set on that queue, but only on the main queue. This eventually leads to a call to dispatch_retain passing null in -[VKRasterOverlayTileSource init] which crashes.

@adriantofan

This comment has been minimized.

Show comment
Hide comment
@adriantofan

adriantofan Apr 22, 2016

Hello, I've run in to the same issue. Can I help fixing it ?
For me it means that UIScheduler is not safe and I have to use QueueScheduler.mainQueueScheduler for UI stuff in order to not dig up unknown bugs.
Why just not make UIScheduler work on the main queue?
Thank you

Hello, I've run in to the same issue. Can I help fixing it ?
For me it means that UIScheduler is not safe and I have to use QueueScheduler.mainQueueScheduler for UI stuff in order to not dig up unknown bugs.
Why just not make UIScheduler work on the main queue?
Thank you

@mdiep

This comment has been minimized.

Show comment
Hide comment
@mdiep

mdiep May 19, 2016

Contributor

Why just not make UIScheduler work on the main queue?

We want to avoid a scheduler hop. That adds a delay that can have unfortunate side effects, and it makes debugging much more difficult.

I'm not sure there's anything we can do about this. You may just need to use the main queue scheduler in some instances.

Contributor

mdiep commented May 19, 2016

Why just not make UIScheduler work on the main queue?

We want to avoid a scheduler hop. That adds a delay that can have unfortunate side effects, and it makes debugging much more difficult.

I'm not sure there's anything we can do about this. You may just need to use the main queue scheduler in some instances.

@ikesyo

This comment has been minimized.

Show comment
Hide comment
@ikesyo

ikesyo May 19, 2016

Member

I've just submitted a PR for this: #2912. Could you try that branch and confirm whether the fix works as expected? @kommen @laptobbe @adriantofan

Member

ikesyo commented May 19, 2016

I've just submitted a PR for this: #2912. Could you try that branch and confirm whether the fix works as expected? @kommen @laptobbe @adriantofan

@ikesyo ikesyo removed the bug label May 19, 2016

@ikesyo ikesyo added the bug label May 27, 2016

@mdiep mdiep closed this in #2912 May 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment