Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

JS client foreverFrame run into script error : Unable to get property 'contentWindow' of undefined or null reference #2249

Closed
Xiaohongt opened this Issue · 3 comments

5 participants

@Xiaohongt
Collaborator

Functional impact:
when JS client use foreverFrame, run connection start ->stop loop, it will run into script error :
SCRIPT5007: Unable to get property 'contentWindow' of undefined or null reference
jquery.signalR.js, line 1642 character 17

in JS library for foreverFrame:

receive: function (connection, data) {
            var cw;

            transportLogic.processMessages(connection, data, connection.onSuccess);
            // Delete the script & div elements
            connection.frameMessageCount = (connection.frameMessageCount || 0) + 1;
            if (connection.frameMessageCount > 50) {
                connection.frameMessageCount = 0;
                cw = connection.frame.contentWindow || connection.frame.contentDocument;
                if (cw && cw.document) {
                    $("body", cw.document).empty();
                }
            }
        },

Repro:
1). you can replace below script in Asp.net Sample HubConnectionAPI.js in :

$(function () {
            "use strict";
            var hubConnectionAPI = $.connection.hubConnectionAPI;                     
            $.connection.hub.logging = true;
            hubConnectionAPI.client.displayMessage = function (value) {
                if (value.indexOf("halo") > 1) {
                    $("<li/>").html("[" + new Date().toTimeString() + "]: " + value).appendTo($("#messages"));
                      $.connection.hub.stop();
                    start();
                }
            }

            $.connection.hub.stateChanged(function (change) {
                var oldState = null,
                    newState = null;
                for (var state in $.signalR.connectionState) {
                    if ($.signalR.connectionState[state] === change.oldState) {
                        oldState = state;
                    } else if ($.signalR.connectionState[state] === change.newState) {
                        newState = state;
                    }
                }

                $("<li/>").html("[" + new Date().toTimeString() + "]  " + oldState + " => " + newState + " " + $.connection.hub.id)
                             .appendTo($("#messages"));
            });

            var start = function () {
                $.connection.hub.start({ transport: activeTransport }).done(function () {                 
                    hubConnectionAPI.server.displayMessageCaller("halo").fail(function (e) {
                        $("<li/>").html("Failed at getMessageCaller: " + e).appendTo($("#messages"));
                    });
                });
            };
            start();
        });

2). request http://localhost:40476/Hubs/HubConnectionAPI/Default.aspx?transport=foreverFrame
3). wait couple mins it run start -> stop loop

Expected result:
no script error

Actual result:

script error :
when connection.frameMessageCount > 50,

SCRIPT5007: Unable to get property 'contentWindow' of undefined or null reference
jquery.signalR.js, line 1642 character 17

@davidfowl
Owner

I did see this error when running your other test

@DamianEdwards

This fundamentally looks like the message received logic can run while the connection is no longer in the connected state. The impact is probably different from transport to transport, but in the case of forever frame the receive logic is just assuming the connection is in the valid state and as such the state fields (i.e. connection.frame) will be present.

Let's try biting the bullet and changing processMessages to schedule the callbacks to be executed via setImmediate (see #1598) rather than executing them inline. This will allow the current stack to complete execution and as such the transport functions' logic should never be impacted by possible changes made to the connection by user callbacks.

This is potentially a risky change so let's do this work in a separate branch and get a build we can use on JabbR to validate it.

@NTaylorMullen NTaylorMullen was assigned
@NTaylorMullen NTaylorMullen referenced this issue from a commit
@NTaylorMullen NTaylorMullen Added checks to verify that we do not try and clear the foreverFrame …
…iframe DOM when we don't need to.

- This involved adding a state check for connected and deleting the frame message count on transport stop.
- Also added an iframeClearThreshold value to the forever frame transport to allow me to test it.

#2249
cce9b0d
@NTaylorMullen NTaylorMullen referenced this issue from a commit
@NTaylorMullen NTaylorMullen Added checks to verify that we do not try and clear the foreverFrame …
…iframe DOM when we don't need to.

- This involved adding a state check for connected and deleting the frame message count on transport stop.
- Also added an iframeClearThreshold value to the forever frame transport to allow me to test it.

#2249
e36f089
@gustavo-armenta

I ran Xiaohong's repro during 20 minutes and it passed, I never got "Failed at getMessageCaller: "

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.