441922 - It should not be possible to have multiple lock requests at the same time

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Dan Mills [:thunder]]

Currently if there is a lock request in process (we are waiting for an XHR response) and lock is called again, the service/dav code will initiate a second lock request.

That is wrong, and in the case where the server is down and requests don't time out, will cause lock requests to stack up.

Comment 1

[Anant Narayanan [:anant]]

Attachment: Disable multiple lock requests

This patch sets an internal _lockAllowed property whenever a lock request is made, and checks if it is not set before actually making the request. This prevents mutiple lock requests from being made at the same time.

We also throw an exception if an attempt to lock is made when another lock request is already progressed. This exception is specially handled by async.js to print only a warning and not the full stack trace.

Here is a sample log from my test of the patch:

2008-06-26 10:27:10	Chrome.Window	INFO	Logging in...
2008-06-26 10:27:10	Chrome.Window	INFO	User string: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0
2008-06-26 10:27:10	Chrome.Window	INFO	Weave version: 0.1.31
2008-06-26 10:27:10	Service.Main	DEBUG	Logging in user test
2008-06-26 10:27:10	Service.Main	INFO	Using server URL: https://services.mozilla.com/0.2/user/test/
2008-06-26 10:27:10	Service.DAV	DEBUG	GET request for meta/version
2008-06-26 10:27:10	Service.Main	INFO	Weave scheduler enabled
2008-06-26 10:27:10	Chrome.Window	INFO	Login successful
2008-06-26 10:28:10	Service.Main	INFO	Running scheduled sync
2008-06-26 10:28:10	Service.DAV	DEBUG	LOCK request for lock
2008-06-26 10:29:10	Service.Main	INFO	Running scheduled sync
2008-06-26 10:29:10	Async.Generator	WARN	Exception: Cannot acquire lock (internal lock)
2008-06-26 10:29:25	Service.DAV	WARN	_makeRequest: got status 0
2008-06-26 10:29:25	Async.Generator	ERROR	Exception: [object Object]
2008-06-26 10:29:25	Async.Generator	DEBUG	Stack trace:
No traceback available.
This exception was raised by an asynchronous coroutine.
Initial async stack trace:
unknown (async) :: WeaveLockWrapper-7
module:wrap.js:91 :: WeaveNotifyWrapper
module:service.js:264 :: WeaveSync__onSchedule
module:util.js:399 :: innerBind
module:util.js:439 :: EL_notify

Note that the second exception is from the first lock request timing out and receiving a status of 0, which must be handled correctly (that's for another bug).

Comment 2

[Dan Mills [:thunder]]

Comment on attachment 326945 [details] [diff] [review]
Disable multiple lock requests

>diff -r 4d2a03483701 modules/async.js
>--- a/modules/async.js	Wed Jun 25 17:11:01 2008 -0700
>+++ b/modules/async.js	Thu Jun 26 10:32:20 2008 -0700
>@@ -192,6 +192,8 @@
> 
>       this._exception = e;
> 
>+    } else if (e.message && e.message == 'Cannot acquire lock (internal lock)') {
>+      this._log.warn("Exception: " + Utils.exceptionStr(e));

r+ except for this change.  I do want us to supress the stack trace, but not in async.js, since it's a weave/dav implementation detail and not related to async.js.

Perhaps we can catch it in the lock wrapper (wrap.js) instead.  But note that it'll be an AsyncException.

Looks good, otherwise.

Comment 3

[Anant Narayanan [:anant]]

Checked-in a995c93356b0. We're going to be looking into exception handling separately.