dsrees/JavaPhoenixClient

Leaking bindings when the channel reply times out?

dustinconrad opened this issue · 3 comments

In PhxPush.kt

/**
     * Starts the Timer which will trigger a timeout after a specific delay
     * in milliseconds is reached.
     */
    fun startTimeout() {
        this.timeoutTimer?.cancel()

        val ref = this.channel.socket.makeRef()
        this.ref = ref

        val refEvent = this.channel.replyEventName(ref)
        this.refEvent = refEvent

        // If a response is received  before the Timer triggers, cancel timer
        // and match the received event to it's corresponding hook.
        this.channel.on(refEvent) {
            this.cancelRefEvent()
            this.cancelTimeout()
            this.receivedMessage = it

            // Check if there is an event status available
            val message = it
            message.status?.let {
                this.matchReceive(it, message)
            }
        }

        // Start the timer. If the timer fires, then send a timeout event to the Push
        this.timeoutTimer = Timer()
        this.timeoutTimer?.schedule(timeout) {
            trigger("timeout", HashMap())
        }
    }

I think what is happenign is that if the timeout actually happens then a Message like PhxMessage(ref=chan_reply_8, topic=, event=, payload={status=timeout}, joinRef=null) is triggered. This is fine. However the bindings created with this.channel.on(refEvent) are never removed.

It should. if chan_reply_8 is received, then cancelRefEvent() will be called which will call this.channel.off(this.refEvent)

Not saying there isn't a bug somewhere, but this is at least the intended logic

The timeout message never triggers the added reply binding is what I am saying. The timeout message looks like PhxMessage(ref=chan_reply_8, topic=, event=, payload={status=timeout}, joinRef=null). Note that the topic and event are empty.

    //In PhxPhush.kt
    /**
     * Triggers an event to be sent through the Channel
     */
    fun trigger(status: String, payload: Payload) {
        val mutPayload = payload.toMutableMap()
        mutPayload["status"] = status

        refEvent?.let {
                                    //ref, topic, event, payload
            val message = PhxMessage(it, "", "", mutPayload)
            this.channel.trigger(message)
        }
    }
    //In PhxChannel.kt
    /**
     * Triggers an event to the correct event binding created by `channel.on("event")
     *
     * @param message: Message that was received that will be sent to the correct binding
     */
    fun trigger(message: PhxMessage) {
        val handledMessage = onMessage(message)
        //the event here is the empty string.  The binding was made for 'chan_reply_8' event.
        this.bindings[message.event]?.forEach { it.second(handledMessage) }
    }

I think if the timeout trigger message was PhxMessage(it, "", refEvent, mutPayload) then things would work as intended. I can open a pull request with a proposed fix and some tests in the next day or two. I wasn't sure what the intended behavior was until your comment.

Resolved in 0.1.8