LockService confirmation I'm doing it right.

897 views
Skip to first unread message

Yagisanatode

unread,
Nov 21, 2020, 8:16:54 PM11/21/20
to Google Apps Script Community
Hi all, 

I just wanted to ensure I have this correct. After testing. It seems okay, but I will be incorporating this into the next tutorial and don't want to mess it up. 

The clientSideForm() is run from the clientside HTML. The inputted script is fairly low stakes and if a lock cannot be achieved, I simply want to return an error to the user (probably, not the actual error code, but something generic).

I've worked mostly with Alen Well's explanation to attempt to get the lock and if successful, do my think serverside and then conclude the lock. Or if it fails to conclude the lock attempts and return a warning. 

LMK if I've missed something. 

~Scott

function clientSideForm(data) {
  const lock = LockService.getScriptLock();

  try {
    lock.waitLock(3000);

    // .... do some stuff then add it to the Google Sheet
    // NOTE! No one will be editing the sheet from the Google Sheets GUI

    SpreadsheetApp.flush(); //Ensure all sheet changes are made.

    lock.releaseLock();

    return data; //Return a value created back to client-side.
  } catch (e) {
    lock.releaseLock();

    return e; //Return error or just an error warning back clientside.
  }
}



 



Adam Morris

unread,
Nov 21, 2020, 8:41:53 PM11/21/20
to google-apps-sc...@googlegroups.com
I actually think it’s better like this:

try {
    lock.waitLock(3000)
} catch (e) {
    // handle it as desired by returning
}

try {
// do stuff
} catch(e){
     // handle error while doing stuff, different from if error happened above (don’t return)
}

SpreadsheetApp.flush()
lock.releaseLock()
return data

The reason I think it’s better is because waitLock might return error but flush nor releaseLock will ever. 

Also if an error happens while doing stuff, you probably need to handle it rather than returning an error right?

Anyway, I think it’s better to have a single line of code in a try block, that way your code know exactly what to do. 

--
You received this message because you are subscribed to the Google Groups "Google Apps Script Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-apps-script-c...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/google-apps-script-community/1e4d79c7-9327-431a-9c77-d6427fcf9a24n%40googlegroups.com.
--

Yagisan Atode

unread,
Nov 22, 2020, 1:00:08 AM11/22/20
to google-apps-sc...@googlegroups.com
Hi Adam, 

Yes, that does make sense. Appreciate the advice. 



You received this message because you are subscribed to a topic in the Google Groups "Google Apps Script Community" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/google-apps-script-community/Vy_gw6Z_0SY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to google-apps-script-c...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/google-apps-script-community/CAHQVb%2BRV0Z578N%2BssMS3yx8tqxLv90F07Pm%2BwikaRPq9Ryjn2w%40mail.gmail.com.

Adam Morris

unread,
Nov 23, 2020, 7:15:58 AM11/23/20
to google-apps-sc...@googlegroups.com
HI there again Scott,

I was inspired by your question to look back at something I had worked on before. I realized that your use case was a pretty good example of a design pattern called a "context manager". So I spent a few hours redoing it, and most of making it less cryptic.


Can you read the readme and let me know if this even makes sense? I tend to complicate things so I'm hoping that it's clear to you how you could use it for this case?

Adam


Yagisan Atode

unread,
Nov 23, 2020, 4:30:27 PM11/23/20
to google-apps-sc...@googlegroups.com
Hi Adam, 

Yeah, it does make some sense, though I had to read it through a few times because I am a bit slow. 

So I'm clear, your 'head' function would set up your work or the environment for your body function. In my example (like you have demonstrated), it is the to set up the LockService.

The setup may fail, but the body will continue to run. 

Then the 'body' does all the work you intend to do within the environment you created in the 'head'.

Finally, 'tail' concludes the setup of the work required of the body. As you say, 'tear down'.

For the dev to determine how they handle any errors you have your error function. 

What I am a little stuck on in my context is why I would wish to continue the 'body' function unless I am testing. I wouldn't want the code to go ahead and add values to the spreadsheet if I can't ensure that the scripted is locked from other executions while my execution is being carried out. I'd just want to throw an error condition and return a failure notice back to the user, right? Especially, now that I know I can do this: 
let swallow = this[_settings_].error.call(state, err) === null; 
😁

 I think I am missing something on this point. 

~Scott

Adam Morris

unread,
Nov 23, 2020, 7:10:58 PM11/23/20
to google-apps-sc...@googlegroups.com
Right, so if an error happens in head, body is not executed.

On this:
What I am a little stuck on in my context is why I would wish to continue the 'body' function unless I am testing. I wouldn't want the code to go ahead and add values to the spreadsheet if I can't ensure that the scripted is locked from other executions while my execution is being carried out. I'd just want to throw an error condition and return a failure notice back to the user, right? Especially, now that I know I can do this: 

The pattern is that the body function is what does the work, which needs set up and tear down.

Thanks for your feedback it gives me a clearer picture of how to design the library.


Pablo Felip

unread,
Nov 24, 2020, 3:05:32 PM11/24/20
to Google Apps Script Community
Hi there, Adam / Scott. Two comments on this...

Don't you think that it would be more advisable to put the clean-up code inside a finally block? I have to admit that I don't do it myself as much as I probably should, but feels safer to me.

} finally {
  SpreadsheetApp.flush();
  lock.releaseLock();
}

Regarding the LockService,  I tend to prefer document locks (getDocumentLock) to script ones. If the script were running as an add-on I understand that a script lock could misbehave as its context would expand beyond a single document, much in the same way as happens when using getUserTriggers or getProjectTriggers() in a document-bound script or in an add-on.

Ah, thanks for your context manager, Adam, I'll examine it carefully, I am sure it will also take me a while to fully get a grasp of it ':-).

Alan Wells

unread,
Nov 24, 2020, 3:47:43 PM11/24/20
to Google Apps Script Community
Good observation about the potential issues with using the Script Lock.
I'd also like to add that if there is a fatal error with ANY of your code,
which prevents the line of code to release the lock from running,
I'm not sure that the lock gets released when that instance of the code stops running.
It should, but I think that I've had issues with that situation before.
Another issue that should be well thought through is the length of the timeout.
In your example, the time out is 3000 milliseconds, or 3 seconds.
What if you have 5 users running the code at the same time?
Should the time out be 5 times the estimated run time of your code?
What happens if 4 instances of your code are "waiting?" 
And they started waiting all at about the same time.
Imagine that instance 1 of 5 has obtained a lock and takes 3 seconds to complete and that the time out is 3 seconds.
Those other 4 instances that are waiting will stop trying to get a lock after 3 seconds.
What is the "downside" of having a long timeout time?
It could be waiting when it doesn't need to if the previous lock somehow doesn't get released.
Once the lock is obtained, there is no setting to release the lock at some time interval.
Either the code releases the lock, or it should be released when that instance of the code has completed,
but I'm not sure whether I trust that to happen.
You don't want the attempt to obtain a lock to be too short or too long, but there is no way to predict that.
I tend to want to "err" on the side of waiting too long, and making sure that the lock will get released by the
code in all possible situations.

Adam Morris

unread,
Nov 24, 2020, 8:27:38 PM11/24/20
to google-apps-sc...@googlegroups.com
Hello again,

I've made some additional improvements to ContextManager, given some of the discussion here. Good thinking about which guard to use, either user level, script level, or document level. It really depends on the use case.

The documentation indicates a lock definitely gets released when script terminates, so unless we have a repeatable test that shows this to be false, we should assume that it does release.

image.png
It also appears to me that the documentation is written as if a script lock is assumed, as can be seen from "you no longer need exclusive access to a section of code" that way of saying sounds like you the developer, but if it's a user lock, it would be "you" the "end user", right?




Alan Wells

unread,
Nov 24, 2020, 9:45:30 PM11/24/20
to Google Apps Script Community
I'm guessing that the most likely case for implementing a User Lock would be something like a Web App that could have multiple users using it at the same time,
AND the code was running as the owner.  It would be interesting to test a Web App with multiple users all running it as the user
Hopefully that would create a separate unique instance for every user that would avoid any conflicts.
Any time that Lock Service is needed, I'd want to test it to make sure that it's working as expected.
Especially if failure had seriously harmful consequences.

Keith Andersen

unread,
Nov 10, 2024, 12:07:51 AM11/10/24
to Google Apps Script Community
Queried Claude on best practice to lock a function to prevent concurrent collision. Here was his solution:

/**
 * Wrapper function that implements a lock to prevent concurrent execution
 * @param {function} fn - The function to be executed with locking
 * @param {number} timeout - Maximum time in seconds to wait for lock (default: 300)
 * @param {string} lockName - Unique name for the lock (default: function name)
 * @return {function} Wrapped function with locking mechanism
 */
function withLock(fn, timeout = 300, lockName) {
  return function (...args) {
    // Get script lock
    const lock = LockService.getScriptLock();
    const actualLockName = lockName || fn.name;
   
    try {
      // Try to acquire lock
      const acquired = lock.tryLock(timeout * 1000);
     
      if (!acquired) {
        throw new Error(`Could not acquire lock for ${actualLockName} after ${timeout} seconds`);
      }
     
      // Execute the wrapped function
      return fn.apply(this, args);
     
    } catch (error) {
      Logger.log(`Error in ${actualLockName}: ${error.message}`);
      throw error;
     
    } finally {
      // Always release the lock in finally block
      try {
        lock.releaseLock();
      } catch (e) {
        // Log but don't throw - the main error is more important
        Logger.log(`Error releasing lock for ${actualLockName}: ${e.message}`);
      }
    }
  };
}

// Example usage:
function processSensitiveData() {
  // Your sensitive operation here
  SpreadsheetApp.getActiveSheet().getRange('A1').setValue(new Date());
}

// Wrap the function with a lock
const safeProcessData = withLock(processSensitiveData, 60, 'processData');

/**
 * Function that can be triggered by multiple users/events
 */
function triggerFunction() {
  try {
    safeProcessData();
  } catch (error) {
    if (error.message.includes('Could not acquire lock')) {
      // Handle lock timeout
      Logger.log('Process already running, please try again later');
    } else {
      // Handle other errors
      throw error;
    }
  }
}

Any thoughts? Goo? Bad? Problem areas?
Keith
Reply all
Reply to author
Forward
0 new messages