pusher / pusher-websocket-dotnet

Pusher Channels Client Library for .NET
MIT License
112 stars 113 forks source link

System.NullReferenceException at PusherClient.Pusher.SubscribeToChannel(ChannelTypes type, String channelName) #50

Closed Boogier closed 4 years ago

Boogier commented 6 years ago

Exception type: System.NullReferenceException Message: Object reference not set to an instance of an object. Stacktrace: at PusherClient.Pusher.SubscribeToChannel(ChannelTypes type, String channelName) at PusherClient.Pusher.Subscribe(String channelName) at PusherClient.Pusher.SubscribeExistingChannels() at PusherClient.Pusher._connection_ConnectionStateChanged(Object sender, ConnectionState state) at PusherClient.Connection.ChangeState(ConnectionState state) at PusherClient.Connection.ParseConnectionEstablished(String data) at PusherClient.Connection.websocket_MessageReceived(Object sender, MessageReceivedEventArgs e) at WebSocket4Net.WebSocket.FireMessageReceived(String message) at WebSocket4Net.Command.Text.ExecuteCommand(WebSocket session, WebSocketCommandInfo commandInfo) at WebSocket4Net.WebSocket.ExecuteCommand(WebSocketCommandInfo commandInfo) at WebSocket4Net.WebSocket.OnDataReceived(Byte[] data, Int32 offset, Int32 length) at WebSocket4Net.WebSocket.client_DataReceived(Object sender, DataEventArgs e) at SuperSocket.ClientEngine.ClientSession.OnDataReceived(Byte[] data, Int32 offset, Int32 length) at SuperSocket.ClientEngine.SslStreamTcpSession.OnDataRead(IAsyncResult result) at System.Net.LazyAsyncResult.Complete(IntPtr userToken) at System.Net.LazyAsyncResult.ProtectedInvokeCallback(Object result, IntPtr userToken) at System.Net.Security._SslStream.ProcessFrameBody(Int32 readBytes, Byte[] buffer, Int32 offset, Int32 count, AsyncProtocolRequest asyncRequest) at System.Net.Security._SslStream.ReadFrameCallback(AsyncProtocolRequest asyncRequest) at System.Net.AsyncProtocolRequest.CompleteRequest(Int32 result) at System.Net.FixedSizeReader.CheckCompletionBeforeNextRead(Int32 bytes) at System.Net.FixedSizeReader.ReadCallback(IAsyncResult transportResult) at System.Net.LazyAsyncResult.Complete(IntPtr userToken) at System.Net.ContextAwareResult.CompleteCallback(Object state) at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state) at System.Net.ContextAwareResult.Complete(IntPtr userToken) at System.Net.LazyAsyncResult.ProtectedInvokeCallback(Object result, IntPtr userToken) at System.Net.Sockets.BaseOverlappedAsyncResult.CompletionPortCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped nativeOverlapped) at System.Threading._IOCompletionCallback.PerformIOCompletionCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped pOVERLAP)

Boogier commented 6 years ago

This happens when network is not so stable. An application crashes as the result of this error.

The issue is supposably caused by _connection variable that can become null in Disconnect() method.

I beleive this is very bad practice to set null value to this variable without using any thread syncing techniques.

Is this really required in Disconnect() method? _connection = null;

Can we remove this line and use follwing check in Connect() method? if (_connection != null && _connection.State != ConnectionState.Disconnected)

imaji commented 6 years ago

Hi,

I'm in the process of cleaning up the code a bit.

I'll check the disconnect method as to the need for nulling the connection. Would you put that check around the connection is already open/exists check?

Thanks, John

Boogier commented 6 years ago

The thing I would do for sure is wrap all _websocket event handlers in try-catch blocks. This at least should protect the app from unavoidable crashes.

McNalYoo commented 6 years ago

Someone find a solution to this problem?

imaji commented 6 years ago

I've popped up a small fix to stop the connection being nulled out. I'm going to look into reworking the whole connection part, as I think it could be improved to make better use of the events coming off of the underlying websocket.

If you have any suggestions, please do feed them back.

Boogier commented 6 years ago

Thank you, imaji. Are we ready to issue nuget package update with this fix?

imaji commented 6 years ago

https://www.nuget.org/packages/PusherClient/1.0.1-beta :)

rh78 commented 4 years ago

Hello we have this issue as well, and it is a huge blocker. It crashes with NullReferenceException at SubscribeToChannel as well, so it seems the issue has not been fixed yet (in 1.1.1).

damdo commented 4 years ago

Similar issue reported in #71

damdo commented 4 years ago

The NullReferenceException should be finally fixed in #95