Reorder lock_volumes and dlock in SD to avoid race conditions

diff --git a/bacula/src/lib/mutex_list.h b/bacula/src/lib/mutex_list.h
index fce166ead5cf0622507319cb6b244ce893f7b15d..5b3e09460dfccf4aa41809593d9391f83bc2c630 100644 (file)
--- a/bacula/src/lib/mutex_list.h
+++ b/bacula/src/lib/mutex_list.h
@@ -34,9 +34,9 @@
  * race conditions and dead locks
  */
 
-#define PRIO_SD_DEV_ACQUIRE   4
-#define PRIO_SD_VOL_LIST      5
-#define PRIO_SD_DEV_ACCESS    10
-#define PRIO_SD_DEV_SPOOL     14
+#define PRIO_SD_DEV_ACQUIRE   4            /* dev.acquire_mutex */
+#define PRIO_SD_DEV_ACCESS    5            /* dev.m_mutex */
+#define PRIO_SD_VOL_LIST      10           /* vol_list_lock */
+#define PRIO_SD_DEV_SPOOL     14           /* dev.spool_mutex */
 
 #endif

diff --git a/bacula/src/lib/plugins.c b/bacula/src/lib/plugins.c
index 26028534d54b25ece578304c2ed62f8d4644d026..70ce223ffdbdf91f944fdc52d9ca59d358fd62a9 100644 (file)
--- a/bacula/src/lib/plugins.c
+++ b/bacula/src/lib/plugins.c
@@ -244,18 +244,17 @@ void dbg_plugin_add_hook(dbg_plugin_hook_t *fct)
 void dbg_print_plugin(FILE *fp)
 {
    Plugin *plugin;
-   fprintf(fp, "Attempt to dump plugins\n");
+   fprintf(fp, "Attempt to dump plugins. Hook count=%d\n", plugin_hook_count);
 
    if (!plugin_list) {
       return;
    }
-
    foreach_alist(plugin, plugin_list) {
       for(int i=0; i < dbg_plugin_hook_count; i++) {
          dbg_plugin_hook_t *fct = dbg_plugin_hooks[i];
          fprintf(fp, "Plugin %p name=\"%s\" disabled=%d\n",
                  plugin, plugin->file, plugin->disabled);
-         fct(plugin, fp);
+//       fct(plugin, fp);
       }
    }
 }

diff --git a/bacula/src/stored/acquire.c b/bacula/src/stored/acquire.c
index d1b41e20ad4f324e9f587ef692253fedc376ea84..4c9f627a1f8825bac5b50b524ca9985e5d68fba8 100644 (file)
--- a/bacula/src/stored/acquire.c
+++ b/bacula/src/stored/acquire.c
@@ -447,13 +447,13 @@ bool release_device(DCR *dcr)
    bool ok = true;
    char tbuf[100];
 
-   lock_volumes();
    dev->dlock();
    if (!dev->is_blocked()) {
       block_device(dev, BST_RELEASING);
    } else if (dev->blocked() == BST_DESPOOLING) {
       dev->set_blocked(BST_RELEASING);
    }
+   lock_volumes();
    Dmsg2(100, "release_device device %s is %s\n", dev->print_name(), dev->is_tape()?"tape":"disk");
 
    /* if device is reserved, job never started, so release the reserve here */
@@ -550,8 +550,8 @@ bool release_device(DCR *dcr)
    Dmsg2(100, "JobId=%u broadcast wait_device_release at %s\n", 
          (uint32_t)jcr->JobId, bstrftimes(tbuf, sizeof(tbuf), (utime_t)time(NULL)));
    pthread_cond_broadcast(&wait_device_release);
-   dev->dunblock(true);
    unlock_volumes();
+   dev->dunblock(true);
    if (dcr->keep_dcr) {
       detach_dcr_from_dev(dcr);
    } else {
@@ -685,8 +685,8 @@ static void locked_detach_dcr_from_dev(DCR *dcr)
 
    /* Detach this dcr only if attached */
    if (dcr->attached_to_dev && dev) {
-      dev->dlock();
       dcr->unreserve_device();
+      dev->dlock();
       dcr->dev->attached_dcrs->remove(dcr);  /* detach dcr from device */
 //    remove_dcr_from_dcrs(dcr);      /* remove dcr from jcr list */
       dev->dunlock();

diff --git a/bacula/src/stored/mount.c b/bacula/src/stored/mount.c
index 462b9b5b4f8f2a994903fc0ef6e5113b21c08ffe..4c022d54df5b9b6985b29a81024ed765edf23dc0 100644 (file)
--- a/bacula/src/stored/mount.c
+++ b/bacula/src/stored/mount.c
@@ -64,6 +64,9 @@ enum {
  * This routine returns a 0 only if it is REALLY
  *  impossible to get the requested Volume.
  *
+ * This routine is entered with the device blocked, but not
+ *   locked.
+ *
  */
 bool DCR::mount_next_write_volume()
 {
@@ -138,6 +141,7 @@ mount_next_vol:
     * and move the tape to the end of data.
     *
     */
+   unlock_volumes();
    if (autoload_device(dcr, true/*writing*/, NULL) > 0) {
       autochanger = true;
       ask = false;
@@ -147,6 +151,7 @@ mount_next_vol:
       ask = retry >= 2;
       do_find = true;           /* do find_a_volume if we retry */
    }
+   lock_volumes();
    Dmsg1(150, "autoload_dev returns %d\n", autochanger);
    /*
     * If we autochanged to correct Volume or (we have not just

diff --git a/bacula/src/stored/reserve.c b/bacula/src/stored/reserve.c
index 677d5ebdeab4ce055ae03d37e0caca07983d0cef..8119f84af7fee6e2bb7672c2374970400e93937e 100644 (file)
--- a/bacula/src/stored/reserve.c
+++ b/bacula/src/stored/reserve.c
@@ -145,6 +145,7 @@ void DCR::clear_reserved()
  */
 void DCR::unreserve_device()
 {
+   dev->dlock();
    lock_volumes();
    if (is_reserved()) {
       clear_reserved();
@@ -162,6 +163,7 @@ void DCR::unreserve_device()
       }
    }
    unlock_volumes();
+   dev->dunlock();
 }
 
 /*