34

I have a function called save(), this function gathers up all the inputs on the page, and performs an AJAX call to the server to save the state of the user's work.

save() is currently called when a user clicks the save button, or performs some other action which requires us to have the most current state on the server (generate a document from the page for example).

I am adding in the ability to auto save the user's work every so often. First I would like to prevent an AutoSave and a User generated save from running at the same time. So we have the following code (I am cutting most of the code and this is not a 1:1 but should be enough to get the idea across):

var isSaving=false;
var timeoutId;
var timeoutInterval=300000;
function save(showMsg)
{
  //Don't save if we are already saving.
  if (isSaving)
  { 
     return;
  }
  isSaving=true;
  //disables the autoSave timer so if we are saving via some other method
  //we won't kick off the timer.
  disableAutoSave();

  if (showMsg) { //show a saving popup}
  params=CollectParams();
  PerformCallBack(params,endSave,endSaveError);

}
function endSave()
{  
    isSaving=false;
    //hides popup if it's visible

    //Turns auto saving back on so we save x milliseconds after the last save.
    enableAutoSave();

} 
function endSaveError()
{
   alert("Ooops");
   endSave();
}
function enableAutoSave()
{
    timeoutId=setTimeOut(function(){save(false);},timeoutInterval);
}
function disableAutoSave()
{
    cancelTimeOut(timeoutId);
}

My question is if this code is safe? Do the major browsers allow only a single thread to execute at a time?

One thought I had is it would be worse for the user to click save and get no response because we are autosaving (And I know how to modify the code to handle this). Anyone see any other issues here?

JoshBerke
  • 66,142
  • 25
  • 126
  • 164
  • So based on that if the user clicks save the timer wouldn't fire until the save function is finished? – JoshBerke Feb 12 '10 at 17:11
  • 1
    If I understand you correctly you're asking if the user has initiated a save, will the setTimeout initiated save ever enter the same method. The answer is no, they won't be in the save() method at the same time, ever. – Jonathon Faust Feb 12 '10 at 17:24
  • What do we do since this information is slightly outdated? JavaScript now has multiple threads: https://developer.mozilla.org/En/Using_web_workers – jamesmortensen Jan 17 '11 at 09:28
  • @jmort - there is still no problem. WebWorkers take a script file as their input, and provide means of communicating between threads. This particular question will not be affected by WebWorkers. – Josh Smeaton Jan 17 '11 at 10:11

5 Answers5

46

JavaScript in browsers is single threaded. You will only ever be in one function at any point in time. Functions will complete before the next one is entered. You can count on this behavior, so if you are in your save() function, you will never enter it again until the current one has finished.

Where this sometimes gets confusing (and yet remains true) is when you have asynchronous server requests (or setTimeouts or setIntervals), because then it feels like your functions are being interleaved. They're not.

In your case, while two save() calls will not overlap each other, your auto-save and user save could occur back-to-back.

If you just want a save to happen at least every x seconds, you can do a setInterval on your save function and forget about it. I don't see a need for the isSaving flag.

I think your code could be simplified a lot:

var intervalTime = 300000;
var intervalId = setInterval("save('my message')", intervalTime);
function save(showMsg)
{
  if (showMsg) { //show a saving popup}
  params=CollectParams();
  PerformCallBack(params, endSave, endSaveError);

  // You could even reset your interval now that you know we just saved.
  // Of course, you'll need to know it was a successful save.
  // Doing this will prevent the user clicking save only to have another
  // save bump them in the face right away because an interval comes up.
  clearInterval(intervalId);
  intervalId = setInterval("save('my message')", intervalTime);
}

function endSave()
{
    // no need for this method
    alert("I'm done saving!");
}

function endSaveError()
{
   alert("Ooops");
   endSave();
}
blong
  • 2,815
  • 8
  • 44
  • 110
Jonathon Faust
  • 12,396
  • 4
  • 50
  • 63
  • Yep and since I have asynchronous calls in save function using the isSaving flag should let me prevent someone else from entering the function until my end save has ran...right? – JoshBerke Feb 12 '10 at 17:12
  • 1
    @Josh I updated my answer a little bit regarding the isSaving flag. All setTimeout does is schedule a function call at least some x milliseconds in the future. It doesn't mean that it will drop what it's doing and run your function -- your scheduled save won't run at the same time as another function. – Jonathon Faust Feb 12 '10 at 17:17
  • Since save makes an asynch call to our server, the isSaving prevents a save from occuring until the EndSave function is called. – JoshBerke Feb 12 '10 at 17:32
  • @Josh That's valid reasoning since it's asynchronous. Are you worried users will slam the save button over and over? The other option is disable the save button until your callback occurs. – Jonathon Faust Feb 12 '10 at 17:35
  • When the user initiates a save we throw up a sort of translucent panel which blocks the user from doing anything. I am more worried about the Auto Save running when the user does something which causes a save to occur – JoshBerke Feb 12 '10 at 17:37
  • @Josh Check out the end of the save function I changed. You can reset your interval when you save so users won't be bugged by an autosave right after they saved. – Jonathon Faust Feb 12 '10 at 17:39
  • @Anurag I don't have a reference since it's not defined in the ECMA specification. From within JavaScript, you can count on the assumption that no two functions run concurrently. Unless you're talking about newer stuff like web workers. Some good explanation from John Resig here: http://ejohn.org/blog/how-javascript-timers-work – Jonathon Faust Jul 28 '11 at 18:17
7

All major browsers only support one javascript thread (unless you use web workers) on a page.

XHR requests can be asynchronous, though. But as long as you disable the ability to save until the current request to save returns, everything should work out just fine.

My only suggestion, is to make sure you indicate to the user somehow when an autosave occurs (disable the save button, etc).

mikefrey
  • 4,653
  • 3
  • 29
  • 40
  • Yep thinking about ways to indicate this to the user great point – JoshBerke Feb 12 '10 at 17:18
  • If you have a gmail account, you'll notice that their approach when autosaving a draft is to grey out the "Save Now" button and change the text to "Saving" and then to "Saved". Only when the user starts making changes to the draft does the "Save Now" button return to a clickable state. – mikefrey Feb 13 '10 at 01:35
2

All the major browsers currently single-thread javascript execution (just don't use web workers since a few browsers support this technique!), so this approach is safe.

For a bunch of references, see Is JavaScript Multithreaded?

Community
  • 1
  • 1
Jeff Sternal
  • 47,787
  • 8
  • 93
  • 120
  • How supported are Web workers? Looks like Firefox 3.5+ haven't found a list showing browser support for this feature. – JoshBerke Feb 12 '10 at 17:33
  • I'm not entirely sure. According to John Resig (here - http://ejohn.org/blog/web-workers/), FireFox 3.5 and Safari 4 (as of July 29, 2009). He also makes the case for structuring your code so that they use web workers when supported (if I read that right). What's more, they appear to be part of the HTML 5 specification (http://www.whatwg.org/specs/web-workers/current-work/) – Jeff Sternal Feb 12 '10 at 17:45
2

Looks safe to me. Javascript is single threaded (unless you are using webworkers)

Its not quite on topic but this post by John Resig covers javascript threading and timers: http://ejohn.org/blog/how-javascript-timers-work/

David Murdoch
  • 87,823
  • 39
  • 148
  • 191
1

I think the way you're handling it is best for your situation. By using the flag you're guaranteeing that the asynchronous calls aren't overlapping. I've had to deal with asynchronous calls to the server as well and also used some sort of flag to prevent overlap.

As others have already pointed out JavaScript is single threaded, but asynchronous calls can be tricky if you're expecting things to say the same or not happen during the round trip to the server.

One thing, though, is that I don't think you actually need to disable the auto-save. If the auto-save tries to happen when a user is saving then the save method will simply return and nothing will happen. On the other hand you're needlessly disabling and reenabling the autosave every time autosave is activated. I'd recommend changing to setInterval and then forgetting about it.

Also, I'm a stickler for minimizing global variables. I'd probably refactor your code like this:

var saveWork = (function() {
  var isSaving=false;
  var timeoutId;
  var timeoutInterval=300000;
  function endSave() {  
      isSaving=false;
      //hides popup if it's visible
  }
  function endSaveError() {
     alert("Ooops");
     endSave();
  }
  function _save(showMsg) {
    //Don't save if we are already saving.
    if (isSaving)
    { 
     return;
    }
    isSaving=true;

    if (showMsg) { //show a saving popup}
    params=CollectParams();
    PerformCallBack(params,endSave,endSaveError);
  }
  return {
    save: function(showMsg) { _save(showMsg); },
    enableAutoSave: function() {
      timeoutId=setInterval(function(){_save(false);},timeoutInterval);
    },
    disableAutoSave: function() {
      cancelTimeOut(timeoutId);
    }
  };
})();

You don't have to refactor it like that, of course, but like I said, I like to minimize globals. The important thing is that the whole thing should work without disabling and reenabling autosave every time you save.

Edit: Forgot had to create a private save function to be able to reference from enableAutoSave

Bob
  • 7,851
  • 5
  • 36
  • 48