ReactiveSwift: Deadlock

Hi,

I am running into a deadlock issue. I’m not entirely sure why it is happening yet. Here is my setup (dumbed down as this is the only part breaking, can provide more if needed).

class ActionController {
	var delay: NSTimeInterval = 0.0
	let undoable = MutableProperty<Bool>(false)

	func start() {
		if delay > 0.0 {
			undoable.value = true
		}
		
		// More stuff
		// Eventually will commit once delay is hit
		// Can provide more if needed.
	}
	
	func undo() {
		guard undoable.value else {
			return
		}
		
		// Do some stuff
		
		/***** DEADLOCK HAPPENS ON LINE BELOW *******/
		undoable.value = false
		/********************************************/
	}
}

class MyViewModel {

	private let currentAction = MutableProperty<ActionController?>(nil)
	
	let undoEnabled = MutableProperty<Bool>(false)
	
	init() {
		undoEnabled <~ currentAction.producer
			.flatMap(.Concat) { action in
				guard let action = action else {
					return SignalProducer<Bool, NoError>(value: false)
				}
				
				return action.undoable.producer
			}
			.skipRepeats()
	}
	
	func commitAction() {
		let action = ActionController()
		action.delay = 10
		action.start()
	}
	
	func undoCurrentAction() {
		currentAction.value?.undo()
	}

	 func onActionCompleted() {
		// Method gets called when ActionController completes    
		currentAction.value = nil
	}

}

Both commitAction() and undoCurrentAction() get called from a UIViewController that the MyViewModel is powering and has subscribers to undoEnabled.

Example for testing:

        viewModel.undoEnabled.producer.startWithNext { [unowned self] enabled in
            if enabled {
                self.viewModel.undoCurrentAction()
            }
        }

The deadlock seems to be happening when calling undo and setting undoable’s value to false.

About this issue

Original URL
State: closed
Created 8 years ago
Comments: 20 (17 by maintainers)

Most upvoted comments

pipe.output.filterMap { $0 == .a ? Event.b : nil }.observe(pipe.input)

Nice example 😃 I also think this kind of data-flow should be accepted, i.e. let’s use recursive lock.

inamiy on Mar 14, 2019

FYI

Queue-drain model without RecursiveLock: https://github.com/ReactiveCocoa/ReactiveSwift/pull/540
- IMO interesting approach, but impl is quite complicated so I still prefer simple RecursiveLock
RxSwift impl: Rx.swift#L89-L122
- Uses RecursiveLock
- fatalError with friendly message when #if DEBUG
- Prints warning message at console when non-DEBUG

Since detecting cyclic dependency is hard in general and requires a lot of runtime test, I prefer the approach RxSwift is taking: prompt a message and use RecursiveLock.

And IMO “discouraged” only works when the problem is predictable and preventable beforehand. But as far as I see many unexpected deadlock examples here, I would say the problem is overall “unpredictable”.

Also, without reentrancy, I think many other potential operators e.g. #308 retryWhen can’t be implemented in ReactiveSwift (unless explicitly using scheduler to move on to async).

inamiy on Mar 15, 2019

Let me give you more concrete example:

class View {
    enum Event {
        case textChanged(text: String)
        case buttonPressed
    }
    private let eventIO = Signal<Event, NoError>.pipe() // or view model
    private let textField = UITextField()
    private let button = UIButton()

    private func bind() {
        textField.reactive.continuousTextValues
            .map { Event.textChanged(text: $0) }
            .observe(eventIO.input)
        button.reactive.controlEvents(.touchUpInside)
            .map { _ in Event.buttonPressed }
            .observe(eventIO.input)
    }

    func observe(action: @escaping (Event) -> Void) {
        eventIO.output.observeValues {
            action($0)
        }
    }
}

class ViewController {
    let someView = View()

    private func bind() {
        someView.observe { [weak self] event in
            switch event {
                case .textChanged(let text): 
                    // do something
                case .buttonPressed:
                    // do something that eventually invoke below
                    self?.view.endEditing(false)
            }
        }
    }
}

When it invokes self?.view.endEditing(false) (or just call textField.resignFirstResponder()) UIKit sometimes sends UITextView.textDidChangeNotification (custom keyboard is related I am assuming) so continuousTextValues sends new value, and that ends up with deadlock. This kind of things can easily happen (ex: Apple changes the event cycle) and it’s too risky to make it deadlock and forcing app to crash.

I do understand pipe.output.filterMap { $0 == .a ? Event.b : nil }.observe(pipe.input) kind of cases should be discouraged though, it actually happens in UI world easily (and sometimes unexpectedly), and I do not see the reason to force them to crash while it works.

What do you think?

chuganzy on Mar 19, 2019