CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 12 of 12

Thread: putting critical section to work

  1. #1
    Join Date
    Aug 2006
    Posts
    515

    putting critical section to work

    I save a file which takes a long time, the time is not an issue right now. While it is being saved, if I press save again, now there are two save operations in progress and there is obviously an assertion.

    I enclosed the save into critical section so the next time user press save, it will not execute that code but this mechanism does not work.
    Code:
    CSingleLock singleLock(&m_cricSection);
    
    if (singleLock.IsLocked())
    {
    AfxMessageBox("locked");
    return;
    }
    singleLock.Lock()
    // ---- saving
    singleLock.Unlock()
    I tried this with CCriticalSection object as well as CEvent, does not work. It just executes the code everytime the control is here.

    How can I make this critical section to work? thanks.

  2. #2
    Join Date
    Feb 2002
    Posts
    4,640

    Re: putting critical section to work

    That code looks correct, just like the MSDN example. Why don't you just gray out the "Save" button during the save? That has the added effect of telling the user that they cannot save yet.

    Viggy

  3. #3
    Arjay's Avatar
    Arjay is offline Moderator / EX MS MVP Power Poster
    Join Date
    Aug 2004
    Posts
    13,312

    Re: putting critical section to work

    Quote Originally Posted by zspirit
    I save a file which takes a long time, the time is not an issue right now. While it is being saved, if I press save again, now there are two save operations in progress and there is obviously an assertion.

    I enclosed the save into critical section so the next time user press save, it will not execute that code but this mechanism does not work.
    Code:
    CSingleLock singleLock(&m_cricSection);
    
    if (singleLock.IsLocked())
    {
    AfxMessageBox("locked");
    return;
    }
    singleLock.Lock()
    // ---- saving
    singleLock.Unlock()
    I tried this with CCriticalSection object as well as CEvent, does not work. It just executes the code everytime the control is here.

    How can I make this critical section to work? thanks.
    This will only work if you are using multi-threading. A critical section will not block if the the same thread attempts to enter the critical section more than one time.

  4. #4
    Join Date
    Aug 2006
    Posts
    515

    Re: putting critical section to work

    Quote Originally Posted by Arjay
    This will only work if you are using multi-threading. A critical section will not block if the the same thread attempts to enter the critical section more than one time.
    Is there anything I can do in case for same thread as well? I thought CSingleLock works for single thread and CMultiLock works for multiple threads!?

    MrViggy,

    I already started working on that as well but had an issue that the Save button on toolbar will not respond on time, the menu option does, both have same IDs. You can have a look at it here.

  5. #5
    Join Date
    Jun 2007
    Location
    New Delhi
    Posts
    70

    Re: putting critical section to work

    I can suggest you a simple idea..you can keep a bool variable which will tell you whether save process is going on or not..if it is going on then do not proceed further and return from there itself..

  6. #6
    Join Date
    Aug 2001
    Location
    Germany
    Posts
    1,384

    Re: putting critical section to work

    Or if it's your own save dialog, disable the button like most websites do.

    Hope this helps,
    Regards,
    Usman.

  7. #7
    Join Date
    Nov 2006
    Posts
    1,611

    Re: putting critical section to work

    My 2 cents worth:

    First, this isn't exactly the purpose for which critical section locks are intended. It could be made to work, but critical section locks are intended to be used for short durations. They'll last as long as you like, but you're involving the OS task scheduler, it's a high speed action (especially when they're nothing else waiting), forming a queue of pending actions.

    That last point means you intend that the subsequent actions, which are held up during the lock, are supposed to execute after the thread currently holding the lock releases. That's not, typically, what you want multiple save requests to do, especially if they're issued while a save is in progress. You typically want the intervening save requests to be ignored, though in some designs you MIGHT want the save to be repeated ONCE, only after the current save is completed, under the assumption that the document contents were changed while a previous save was in progress ( a dubious design notion, but perhaps it's possible).

    So, you don't really want locks via critical section, you want something that BLOCKS. The notion of disabling the save button is closer to the correct design decision.

    Imagine what would happen, for example, if your user hit 10 or 20 save button clicks while the first save is in progress. If you spun a thread for each save response, that document would save 20 times in a row (19 of which would probably be for no reason).

    What you want is a state (previously suggested with a simple bool), that indicates a save is currently in progress. THAT boolean will need protection from a critical section (you lock the critical section before checking the boolean, if it's set to 'saving', you unlock and skip the save - otherwise you set the boolean, unlock, and save - yes - save while the critical section is unlocked, the boolean stops competing saves).

    Then, don't forget to reset the boolean (under critical section lock) when the save is completed, so subsequent saves work.

    Disabling the 'save' button mirrors the state of this boolean (if a save is in progress, the save button click makes no sense).
    If my post was interesting or helpful, perhaps you would consider clicking the 'rate this post' to let me know (middle icon of the group in the upper right of the post).

  8. #8
    Arjay's Avatar
    Arjay is offline Moderator / EX MS MVP Power Poster
    Join Date
    Aug 2004
    Posts
    13,312

    Re: putting critical section to work

    Does this application use menu items for the save operation or a button?

    If a button, just add a control variable for the button (in VC7.1 and above, right click on the button, choose add variable, enter a name and click finish; In VC6, use the class wizard). Next, in the save button handler call EnableWindow to disable and enable the button.

    Code:
    void CSaveDlg::OnBnClickedSave()
    {
    	// Disable the Save button
    	m_btnSave.EnableWindow( FALSE );
    
    	// Put the save code here
    
    	// Enable the Save button
    	m_btnSave.EnableWindow( TRUE );
    }
    It's that easy.

    Btw, when you right click on the button and choose 'Add Variable' to add the control variable, a CButton variable gets added to the class declaration and the following DoDataExchange entry gets created.

    Code:
    void CSaveDlg::DoDataExchange(CDataExchange* pDX)
    {
    	CDialog::DoDataExchange(pDX);
    	DDX_Control(pDX, ID_SAVE, m_btnSave);
    }
    Last edited by Arjay; November 7th, 2007 at 01:48 PM.

  9. #9
    Join Date
    Aug 2006
    Posts
    515

    Re: putting critical section to work

    Quote Originally Posted by JVene
    THAT boolean will need protection from a critical section (you lock the critical section before checking the boolean, if it's set to 'saving', you unlock and skip the save - otherwise you set the boolean, unlock, and save - yes - save while the critical section is unlocked, the boolean stops competing saves).
    I agree with the rest of your comments, they are convincing but I think if there is only one therad we probably don't need to protect that bool in critical section block as well. In my like most GUI applications there is only one GUI thread - there are other worker threads but they could never call Save.

  10. #10
    Join Date
    Aug 2006
    Posts
    515

    Re: putting critical section to work

    Quote Originally Posted by Arjay
    Code:
    void CSaveDlg::OnBnClickedSave()
    {
    	// Disable the Save button
    	m_btnSave.EnableWindow( FALSE );
    
    	// Put the save code here
    
    	// Enable the Save button
    	m_btnSave.EnableWindow( TRUE );
    }
    This is an interesting solution, but the button in my case is in toolbar. So only if there could have been anything like GetDlgItem() which could get a pointer to the button in toolbar and this would have been a good solution.

    I don't think CToolBar has any member functions either which could help or is there any way one can get the button pointer from toolbar or just set the state?

  11. #11
    Join Date
    Nov 2006
    Posts
    1,611

    Re: putting critical section to work

    there are other worker threads but they could never call Save
    That may be true in your design, and as such, critical sections wouldn't be required at all (even when attempting to block multiple saves - only one thread could every interact with that critical section).

    Now that I consider a single threaded design, it appears the only way multiple save commands COULD come through would be from a queue of the commands, meaning there would never be a lock or even a block unless you disable the save command itself (the thread would never be available to process a save button click unless the save were finished).

    However, I wonder why you wouldn't place the save into a thread? It may be as well, if there's nothing your user CAN do while the save is under process, but a file save is one of the more typical reasons TO thread.

    It does complicate the design some, so if there's limited benefit, then you should keep your current design. If you have an MDI or the more modern tabbed document design, or if there's anything else the user might be able to do while a save is under way, and if (if's are pilling up now) the save might take more than a second or two, saving in a thread is highly desirable. It does mean you have to postpone WM_CLOSE requests for the application, carefully handle all the related functions which can't be performed on the document itself while it's locked during saving (again a boolean state), etc. The user does appreciate the effort when it comes into play, though.
    If my post was interesting or helpful, perhaps you would consider clicking the 'rate this post' to let me know (middle icon of the group in the upper right of the post).

  12. #12
    Join Date
    Nov 2006
    Posts
    1,611

    Re: putting critical section to work

    Sorry to double post, this is a separate concept though.

    I don't think CToolBar has any member functions either which could help or is there any way one can get the button pointer from toolbar or just set the state?
    If you're using standard MFC, the toolbar and menu should respond to ON_UPDATE_COMMAND_UI mechanisms.
    If my post was interesting or helpful, perhaps you would consider clicking the 'rate this post' to let me know (middle icon of the group in the upper right of the post).

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Windows Mobile Development Center


Click Here to Expand Forum to Full Width




On-Demand Webinars (sponsored)