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

Leaking bindings when the channel reply times out? #36

Closed
dustinconrad opened this issue Feb 15, 2019 · 3 comments
Closed

Leaking bindings when the channel reply times out? #36

dustinconrad opened this issue Feb 15, 2019 · 3 comments

Comments

@dustinconrad
Copy link
Contributor

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.

@dsrees
Copy link
Owner

dsrees commented Feb 17, 2019

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

@dustinconrad
Copy link
Contributor Author

dustinconrad commented Feb 17, 2019

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.

@dsrees
Copy link
Owner

dsrees commented Mar 4, 2019

Resolved in 0.1.8

@dsrees dsrees closed this as completed Mar 4, 2019
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