ArgumentNullException in SessionStateProvider

We just ran into this error (multiple times):

exception : "System.Web.HttpException (0x80004005): Exception of type 'System.Web.HttpException' was thrown. ---> System.ArgumentNullException: Buffer cannot be null. NEWLINETOKEN Parameter name: buffer NEWLINETOKEN at System.IO.MemoryStream..ctor(Byte[] buffer, Boolean writable) NEWLINETOKEN at System.IO.MemoryStream..ctor(Byte[] buffer) NEWLINETOKEN at Couchbase.AspNet.SessionState.CouchbaseSessionStateProvider.SessionStateItem.Load(String headerPrefix, String dataPrefix, IBucket bucket, String id, Boolean metaOnly) NEWLINETOKEN at Couchbase.AspNet.SessionState.CouchbaseSessionStateProvider.SessionStateItem.Load(IBucket bucket, String id, Boolean metaOnly) NEWLINETOKEN at Couchbase.AspNet.SessionState.CouchbaseSessionStateProvider.GetSessionStoreItem(IBucket bucket, HttpContext context, Boolean acquireLock, String id, Boolean& locked, TimeSpan& lockAge, Object& lockId, SessionStateActions& actions) NEWLINETOKEN at Couchbase.AspNet.SessionState.CouchbaseSessionStateProvider.GetItemExclusive(HttpContext context, String id, Boolean& locked, TimeSpan& lockAge, Object& lockId, SessionStateActions& actions) NEWLINETOKEN at System.Web.SessionState.SessionStateModule.GetSessionStateItem() NEWLINETOKEN at System.Web.SessionState.SessionStateModule.PollLockedSessionCallback(Object state) NEWLINETOKEN at System.Web.HttpAsyncResult.End() NEWLINETOKEN at System.Web.SessionState.SessionStateModule.EndAcquireState(IAsyncResult ar) NEWLINETOKEN at System.Web.HttpApplication.AsyncEventExecutionStep.OnAsyncEventCompletion(IAsyncResult ar)",

This is presumably happening because that Load() function gets the header document and then checks to see if Status == KeyNotFound, but doesn’t check to see if it failed for any other reason. Then a scenario where the Get() fails for some other reason happens, thus resulting in a null object being sent into MemoryStream(). Should the header document check to see if the value == null similar to how it checks the session document in the same function? Or at least check value.Success?


Patrick Davis

Also forgot to point out: this is on 2.0.0-beta2. The lines of code I’m referring to are here: https://github.com/couchbaselabs/couchbase-aspnet/blob/2.0/Couchbase.AspNet/SessionState/CouchbaseSessionStateProvider.cs#L309

And I’m also realizing this is a duplicate of AspNet incident (CBASP-16?) :slight_smile:

1 Like

@pdavis -

I made a ticket for this: https://issues.couchbase.com/browse/CBASP-21

Not sure exactly, but the current behavior is not correct. If you feel inclined, try to fix and push a pull request. If you do end up working on it, set the ticket to in-progress so that were not duplicating efforts here :slight_smile:

Thanks,

Jeff

@pdavis -

There is a commit in this PR that looks like it fixes the issue: https://github.com/couchbaselabs/couchbase-aspnet/pull/4/files

I’ll see if I can pull it out and submit it alone with a few other commits from that PR. I can’t take the whole PR, because there are some things not quite right with it.

-Jeff

2 Likes

Oh, this looks like it could help the final release, thanks!

A few points on the error handling provided:

The retries will occur 10 times, sleeping for a total time of 30s. That’s an awful long time for the AspNet Provider to hang on to a request and not return. These values (# of retries & sleep time) should be configurable. A common pattern for retry sleeps is to increase the time per retry. Harder to make that configurable, I acknowledge. Scrolling down, there’s some other concerning retry count code as well.

Should ResponseStatus.Busy be included in the retry status codes?

What’s the guidance on how long it takes these managed status codes to resolve? Typically should it be “one and done” such that a VBucketBelongsToAnotherServer should only occur once? Does the client automatically get a new Cluster map when that’s returned? I suspect (but don’t w/o logging) that VBucketBelongsToAnotherServer is the problem we’re running into.

if (header == null || header.Success == false || header.Status == ResponseStatus.KeyNotFound)
It’s unnecessary to header != null and also for KeyNotFound here.

The LoadBody() method has similar issues (retries, etc). Should be protected if not private. Perhaps that code with retry logic should be pulled out and made common.

As alluded to above, no error logging. Logging would definitely help here.

Thanks!

-H

Yeah, that is one of the issues I had with the commit.

Well, it’s an indication that the server I having issues. I retry here may succeed or simply put more pressure on the server. An exponential backoff/retry might make sense, however.

The client itself should never return a NMV; the result of a NMV will always lead to an OperationTimeout because the client will internally try to resolve the NMV (by checking for cluster map changes) and retrying the operation. The session provider is a consumer of the client and can have it’s own logic for handling OperationTimeouts, however.

Yeah, I can add this as a task. The client has it’s own logging, but there is nothing for the provider and it would be useful for tracking bugs, etc.

Thanks for the feedback,

-Jeff

2 Likes

@pdavis, @unhuman -

Beta 3 was published to NuGet last Friday. You’ll want to read the release notes on the github page because there are some breaking changes between Beta 2 and Beta 3, hopefully the changes are fairly easy to make. A blog is forthcoming.

Please feel free to query me with questions or provide feedback.

Thanks,

Jeff

1 Like

@jmorris,

Still using beta2 and this issue cropped up again today during a deployment. Hadn’t seen it in quite some time. In any event, there’s some concern that the fix in beta3/4, as provided, will log off users rather than hard fail. Still trying to investigate over here, but that prompted a review of latest and that thought.

BTW, this got “fixed” by issuing an ‘iisreset’. The server was in a bad state (all failed) until that occurred.

Thanks - H