Skip to content

Commit

Permalink
Change long polling request to use same URL and POST instead of GET
Browse files Browse the repository at this point in the history
- Update client protocol
- This change is intended to avoid a memory leak in IE/Chrome

#2953
  • Loading branch information
Xiaohongt committed Jun 9, 2014
1 parent 1e142c4 commit 267e185
Show file tree
Hide file tree
Showing 14 changed files with 178 additions and 58 deletions.
4 changes: 2 additions & 2 deletions src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.core.js
Expand Up @@ -100,7 +100,7 @@
isDisconnecting = function (connection) {
return connection.state === signalR.connectionState.disconnected;
},

supportsKeepAlive = function (connection) {
return connection._.keepAliveData.activated &&
connection.transport.supportsKeepAlive(connection);
Expand Down Expand Up @@ -403,7 +403,7 @@

state: signalR.connectionState.disconnected,

clientProtocol: "1.4",
clientProtocol: "1.5",

reconnectDelay: 2000,

Expand Down
Expand Up @@ -206,13 +206,14 @@
throw new Error("Query string property must be either a string or object.");
},

getUrl: function (connection, transport, reconnecting, poll) {
// BUG #2953: The url needs to be same otherwise it will cause a memory leak
getUrl: function (connection, transport, reconnecting, poll, ajaxPost) {
/// <summary>Gets the url for making a GET based connect request</summary>
var baseUrl = transport === "webSockets" ? "" : connection.baseUrl,
url = baseUrl + connection.appRelativeUrl,
qs = "transport=" + transport;

if (connection.groupsToken) {
if (!ajaxPost && connection.groupsToken) {
qs += "&groupsToken=" + window.encodeURIComponent(connection.groupsToken);
}

Expand All @@ -226,13 +227,17 @@
url += "/reconnect";
}

if (connection.messageId) {
if (!ajaxPost && connection.messageId) {
qs += "&messageId=" + window.encodeURIComponent(connection.messageId);
}
}
url += "?" + qs;
url = transportLogic.prepareQueryString(connection, url);
url += "&tid=" + Math.floor(Math.random() * 11);

if (!ajaxPost) {
url += "&tid=" + Math.floor(Math.random() * 11);
}

return url;
},

Expand Down
Expand Up @@ -90,7 +90,16 @@
connect = (messageId === null),
reconnecting = !connect,
polling = !raiseReconnect,
url = transportLogic.getUrl(instance, that.name, reconnecting, polling);
url = transportLogic.getUrl(instance, that.name, reconnecting, polling, true /* use Post for longPolling */),
postData = {};

if (instance.messageId) {
postData.messageId = instance.messageId;
}

if (instance.groupsToken) {
postData.groupsToken = instance.groupsToken;
}

// If we've disconnected during the time we've tried to re-instantiate the poll then stop.
if (isDisconnecting(instance) === true) {
Expand All @@ -105,6 +114,9 @@
}
},
url: url,
type: "POST",
contentType: signalR._.defaultContentType,
data: postData,
success: function (result) {
var minData,
delay = 0,
Expand Down
Expand Up @@ -12,7 +12,7 @@ public class ProtocolResolver
private readonly Version _minimumDelayedStartVersion = new Version(1, 4);

public ProtocolResolver() :
this(new Version(1, 2), new Version(1, 4))
this(new Version(1, 2), new Version(1, 5))
{
}

Expand Down
37 changes: 22 additions & 15 deletions src/Microsoft.AspNet.SignalR.Core/PersistentConnection.cs
Expand Up @@ -181,7 +181,7 @@ public Task ProcessRequest(IDictionary<string, object> environment)
/// Thrown if the transport wasn't specified.
/// Thrown if the connection id wasn't specified.
/// </exception>
public virtual Task ProcessRequest(HostContext context)
public virtual async Task ProcessRequest(HostContext context)
{
if (context == null)
{
Expand All @@ -195,35 +195,40 @@ public virtual Task ProcessRequest(HostContext context)

if (IsNegotiationRequest(context.Request))
{
return ProcessNegotiationRequest(context);
await ProcessNegotiationRequest(context).PreserveCulture();
return;
}
else if (IsPingRequest(context.Request))
{
return ProcessPingRequest(context);
await ProcessPingRequest(context).PreserveCulture();
return;
}

Transport = GetTransport(context);

if (Transport == null)
{
return FailResponse(context.Response, String.Format(CultureInfo.CurrentCulture, Resources.Error_ProtocolErrorUnknownTransport));
await FailResponse(context.Response, String.Format(CultureInfo.CurrentCulture, Resources.Error_ProtocolErrorUnknownTransport)).PreserveCulture();
return;
}

string connectionToken = context.Request.QueryString["connectionToken"];

// If there's no connection id then this is a bad request
if (String.IsNullOrEmpty(connectionToken))
{
return FailResponse(context.Response, String.Format(CultureInfo.CurrentCulture, Resources.Error_ProtocolErrorMissingConnectionToken));
await FailResponse(context.Response, String.Format(CultureInfo.CurrentCulture, Resources.Error_ProtocolErrorMissingConnectionToken)).PreserveCulture();
return;
}

string connectionId;
string message;
int statusCode;

if (!TryGetConnectionId(context, connectionToken, out connectionId, out message, out statusCode))
{
return FailResponse(context.Response, message, statusCode);
await FailResponse(context.Response, message, statusCode).PreserveCulture();
return;
}

// Set the transport's connection id to the unprotected one
Expand All @@ -232,8 +237,11 @@ public virtual Task ProcessRequest(HostContext context)
// Get the user id from the request
string userId = UserIdProvider.GetUserId(context.Request);

// Get the groups oken from the request
string groupsToken = await Transport.GetGroupsToken().PreserveCulture();

IList<string> signals = GetSignals(userId, connectionId);
IList<string> groups = AppendGroupPrefixes(context, connectionId);
IList<string> groups = AppendGroupPrefixes(context, connectionId, groupsToken);

Connection connection = CreateConnection(connectionId, signals, groups);

Expand All @@ -245,7 +253,8 @@ public virtual Task ProcessRequest(HostContext context)
// because ProcessStartRequest calls OnConnected.
if (IsStartRequest(context.Request))
{
return ProcessStartRequest(context, connectionId);
await ProcessStartRequest(context, connectionId).PreserveCulture();
return;
}

Transport.Connected = () =>
Expand Down Expand Up @@ -277,7 +286,7 @@ public virtual Task ProcessRequest(HostContext context)
}
};

return Transport.ProcessRequest(connection).OrEmpty().Catch(Counters.ErrorsAllTotal, Counters.ErrorsAllPerSec);
await Transport.ProcessRequest(connection).OrEmpty().Catch(Counters.ErrorsAllTotal, Counters.ErrorsAllPerSec).PreserveCulture();
}

[SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "We want to catch any exception when unprotecting data.")]
Expand Down Expand Up @@ -328,10 +337,8 @@ public virtual Task ProcessRequest(HostContext context)
}

[SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "We want to prevent any failures in unprotecting")]
internal IList<string> VerifyGroups(HostContext context, string connectionId)
internal IList<string> VerifyGroups(string connectionId, string groupsToken)
{
string groupsToken = context.Request.QueryString["groupsToken"];

if (String.IsNullOrEmpty(groupsToken))
{
return ListHelper<string>.Empty;
Expand Down Expand Up @@ -366,9 +373,9 @@ internal IList<string> VerifyGroups(HostContext context, string connectionId)
return JsonSerializer.Parse<string[]>(groupsValue);
}

private IList<string> AppendGroupPrefixes(HostContext context, string connectionId)
private IList<string> AppendGroupPrefixes(HostContext context, string connectionId, string groupsToken)
{
return (from g in OnRejoiningGroups(context.Request, VerifyGroups(context, connectionId), connectionId)
return (from g in OnRejoiningGroups(context.Request, VerifyGroups(connectionId, groupsToken), connectionId)
select GroupPrefix + g).ToList();
}

Expand Down
29 changes: 7 additions & 22 deletions src/Microsoft.AspNet.SignalR.Core/Transports/ForeverTransport.cs
Expand Up @@ -19,7 +19,6 @@ public abstract class ForeverTransport : TransportDisconnectBase, ITransport

private readonly IPerformanceCounterManager _counters;
private readonly JsonSerializer _jsonSerializer;
private string _lastMessageId;
private IDisposable _busRegistration;

internal RequestLifetime _transportLifetime;
Expand Down Expand Up @@ -52,19 +51,6 @@ protected virtual int MaxMessages
}
}

protected string LastMessageId
{
get
{
if (_lastMessageId == null)
{
_lastMessageId = Context.Request.QueryString["messageId"];
}

return _lastMessageId;
}
}

protected JsonSerializer JsonSerializer
{
get { return _jsonSerializer; }
Expand Down Expand Up @@ -92,32 +78,31 @@ protected virtual void OnSendingResponse(PersistentResponse response)
internal Action BeforeReceive;
internal Action<Exception> AfterRequestEnd;

protected override void InitializePersistentState()
protected override async Task InitializePersistentState()
{
base.InitializePersistentState();
await base.InitializePersistentState().PreserveCulture();

// The _transportLifetime must be initialized after calling base.InitializePersistentState since
// _transportLifetime depends on _requestLifetime.
_transportLifetime = new RequestLifetime(this, _requestLifeTime);
}

protected Task ProcessRequestCore(ITransportConnection connection)
protected async Task ProcessRequestCore(ITransportConnection connection)
{
Connection = connection;

if (IsSendRequest)
{
return ProcessSendRequest();
await ProcessSendRequest().PreserveCulture();
}
else if (IsAbortRequest)
{
return Connection.Abort(ConnectionId);
await Connection.Abort(ConnectionId).PreserveCulture();
}
else
{
InitializePersistentState();

return ProcessReceiveRequest(connection);
await InitializePersistentState().PreserveCulture();
await ProcessReceiveRequest(connection).PreserveCulture();
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/Microsoft.AspNet.SignalR.Core/Transports/ITransport.cs
Expand Up @@ -4,6 +4,8 @@
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.AspNet.SignalR.Infrastructure;
using Microsoft.AspNet.SignalR.Hosting;
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNet.SignalR.Transports
{
Expand Down Expand Up @@ -37,6 +39,13 @@ public interface ITransport
/// </summary>
string ConnectionId { get; set; }

/// <summary>
/// Get groupsToken in request over the transport.
/// </summary>
/// <returns>groupsToken in request</returns>
[SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate", Justification = "This is for async.")]
Task<string> GetGroupsToken();

/// <summary>
/// Processes the specified <see cref="ITransportConnection"/> for this transport.
/// </summary>
Expand Down
Expand Up @@ -9,6 +9,7 @@
using Microsoft.AspNet.SignalR.Json;
using Microsoft.AspNet.SignalR.Tracing;
using Newtonsoft.Json;
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNet.SignalR.Transports
{
Expand Down Expand Up @@ -94,6 +95,30 @@ protected override bool SuppressReconnect
}
}

protected override async Task InitializeMessageId()
{
_lastMessageId = Context.Request.QueryString["messageId"];

if (_lastMessageId == null)
{
var form = await Context.Request.ReadForm().PreserveCulture();
_lastMessageId = form["messageId"];
}
}

[SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate", Justification = "This is for async.")]
public override async Task<string> GetGroupsToken()
{
var groupsToken = Context.Request.QueryString["groupsToken"];

if (groupsToken == null)
{
var form = await Context.Request.ReadForm().PreserveCulture();
groupsToken = form["groupsToken"];
}
return groupsToken;
}

public override Task KeepAlive()
{
// Ensure delegate continues to use the C# Compiler static delegate caching optimization.
Expand Down Expand Up @@ -250,7 +275,7 @@ private static Task PerformCompleteSend(object state)

return PerformPartialSend(state);
}

private void AddTransportData(PersistentResponse response)
{
if (_configurationManager.LongPollDelay != TimeSpan.Zero)
Expand Down

0 comments on commit 267e185

Please sign in to comment.