Merge pull request #23 from jackaudio/device_reservation_fixes

Device reservation fixes

dbus/audio_reserve.c

@@ -76,6 +76,8 @@ SERVER_EXPORT bool audio_acquire(const char * device_name)
 
     assert(gReserveCount < DEVICE_MAX);
 
+    dbus_error_init(&error);
+
     if ((ret= rd_acquire(
                  &gReservedDevice[gReserveCount].reserved_device,
                  gConnection,
@@ -86,6 +88,7 @@ SERVER_EXPORT bool audio_acquire(const char * device_name)
                  &error)) < 0) {
 
         jack_error("Failed to acquire device name : %s error : %s", device_name, (error.message ? error.message : strerror(-ret)));
+        dbus_error_free(&error);
         return false;
     }
 

dbus/controller.c

@@ -377,6 +377,8 @@ on_device_acquire(const char * device_name)
     int ret;
     DBusError error;
 
+    dbus_error_init(&error);
+
     ret = rd_acquire(
         &g_reserved_device[g_device_count].reserved_device,
         g_connection,
@@ -388,6 +390,7 @@ on_device_acquire(const char * device_name)
     if (ret  < 0)
     {
         jack_error("Failed to acquire device name : %s error : %s", device_name, (error.message ? error.message : strerror(-ret)));
+        dbus_error_free(&error);
         return false;
     }
 

dbus/reserve.c

@@ -31,6 +31,11 @@
 #include <stdint.h>
 
 #include "reserve.h"
+#include "jack/control.h"
+
+#define RESERVE_ERROR_NO_MEMORY            "org.freedesktop.ReserveDevice1.Error.NoMemory"
+#define RESERVE_ERROR_PROTOCOL_VIOLATION   "org.freedesktop.ReserveDevice1.Error.Protocol"
+#define RESERVE_ERROR_RELEASE_DENIED       "org.freedesktop.ReserveDevice1.Error.ReleaseDenied"
 
 struct rd_device {
 	int ref;
@@ -371,34 +376,51 @@ int rd_acquire(
 	dbus_bool_t good;
     vtable.message_function = object_handler;
 
-	if (!error)
+	if (!error) {
 		error = &_error;
+		dbus_error_init(error);
+	}
 
-	dbus_error_init(error);
-
-	if (!_d)
-		return -EINVAL;
+	if (!_d) {
+		assert(0);
+		r = -EINVAL;
+		goto fail;
+	}
 
-	if (!connection)
-		return -EINVAL;
+	if (!connection) {
+		assert(0);
+		r = -EINVAL;
+		goto fail;
+	}
 
-	if (!device_name)
-		return -EINVAL;
+	if (!device_name) {
+		assert(0);
+		r = -EINVAL;
+		goto fail;
+	}
 
-	if (!request_cb && priority != INT32_MAX)
-		return -EINVAL;
+	if (!request_cb && priority != INT32_MAX) {
+		assert(0);
+		r = -EINVAL;
+		goto fail;
+	}
 
-	if (!(d = (rd_device *)calloc(sizeof(rd_device), 1)))
-		return -ENOMEM;
+	if (!(d = (rd_device *)calloc(sizeof(rd_device), 1))) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot allocate memory for rd_device struct");
+		r = -ENOMEM;
+		goto fail;
+	}
 
 	d->ref = 1;
 
 	if (!(d->device_name = strdup(device_name))) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot duplicate device name string");
 		r = -ENOMEM;
 		goto fail;
 	}
 
 	if (!(d->application_name = strdup(application_name))) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot duplicate application name string");
 		r = -ENOMEM;
 		goto fail;
 	}
@@ -408,12 +430,14 @@ int rd_acquire(
 	d->request_cb = request_cb;
 
 	if (!(d->service_name = (char*)malloc(sizeof(SERVICE_PREFIX) + strlen(device_name)))) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot allocate memory for service name string");
 		r = -ENOMEM;
 		goto fail;
 	}
 	sprintf(d->service_name, SERVICE_PREFIX "%s", d->device_name);
 
 	if (!(d->object_path = (char*)malloc(sizeof(OBJECT_PREFIX) + strlen(device_name)))) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot allocate memory for object path string");
 		r = -ENOMEM;
 		goto fail;
 	}
@@ -425,20 +449,28 @@ int rd_acquire(
 		     DBUS_NAME_FLAG_DO_NOT_QUEUE|
 		     (priority < INT32_MAX ? DBUS_NAME_FLAG_ALLOW_REPLACEMENT : 0),
 		     error)) < 0) {
+		jack_error("dbus_bus_request_name() failed. (1)");
 		r = -EIO;
 		goto fail;
 	}
 
-	if (k == DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER || k == DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER)
+	switch (k) {
+	case DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER:
+	case DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER:
 		goto success;
-
-	if (k != DBUS_REQUEST_NAME_REPLY_EXISTS) {
+	case DBUS_REQUEST_NAME_REPLY_EXISTS:
+		break;
+	case DBUS_REQUEST_NAME_REPLY_IN_QUEUE : /* DBUS_NAME_FLAG_DO_NOT_QUEUE was specified */
+	default:								/* unknown reply returned */
+		jack_error("request name reply with unexpected value %d.", k);
+		assert(0);
 		r = -EIO;
 		goto fail;
 	}
 
 	if (priority <= INT32_MIN) {
 		r = -EBUSY;
+		dbus_set_error(error, RESERVE_ERROR_RELEASE_DENIED, "Device reservation request with priority %"PRIi32" denied for \"%s\"", priority, device_name);
 		goto fail;
 	}
 
@@ -447,6 +479,7 @@ int rd_acquire(
 		      d->object_path,
 		      "org.freedesktop.ReserveDevice1",
 		      "RequestRelease"))) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot allocate memory for RequestRelease method call");
 		r = -ENOMEM;
 		goto fail;
 	}
@@ -455,6 +488,7 @@ int rd_acquire(
 		    m,
 		    DBUS_TYPE_INT32, &d->priority,
 		    DBUS_TYPE_INVALID)) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot append args for RequestRelease method call");
 		r = -ENOMEM;
 		goto fail;
 	}
@@ -469,10 +503,12 @@ int rd_acquire(
 		    dbus_error_has_name(error, DBUS_ERROR_UNKNOWN_METHOD) ||
 		    dbus_error_has_name(error, DBUS_ERROR_NO_REPLY)) {
 			/* This must be treated as denied. */
+			jack_info("Device reservation request with priority %"PRIi32" denied for \"%s\": %s (%s)", priority, device_name, error->name, error->message);
 			r = -EBUSY;
 			goto fail;
 		}
 
+		jack_error("dbus_connection_send_with_reply_and_block(RequestRelease) failed.");
 		r = -EIO;
 		goto fail;
 	}
@@ -482,11 +518,13 @@ int rd_acquire(
 		    error,
 		    DBUS_TYPE_BOOLEAN, &good,
 		    DBUS_TYPE_INVALID)) {
+		jack_error("RequestRelease() reply is invalid.");
 		r = -EIO;
 		goto fail;
 	}
 
 	if (!good) {
+        dbus_set_error(error, RESERVE_ERROR_RELEASE_DENIED, "Device reservation request with priority %"PRIi32" denied for \"%s\" via RequestRelease()", priority, device_name);
 		r = -EBUSY;
 		goto fail;
 	}
@@ -498,11 +536,14 @@ int rd_acquire(
 		     (priority < INT32_MAX ? DBUS_NAME_FLAG_ALLOW_REPLACEMENT : 0)|
 		     DBUS_NAME_FLAG_REPLACE_EXISTING,
 		     error)) < 0) {
+		jack_error("dbus_bus_request_name() failed. (2)");
 		r = -EIO;
 		goto fail;
 	}
 
 	if (k != DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) {
+        /* this is racy, another contender may have acquired the device */
+		dbus_set_error(error, RESERVE_ERROR_PROTOCOL_VIOLATION, "request name reply is not DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER but %d.", k);
 		r = -EIO;
 		goto fail;
 	}
@@ -510,11 +551,13 @@ int rd_acquire(
 success:
 	d->owning = 1;
 
-	if (!(dbus_connection_register_object_path(
+	if (!(dbus_connection_try_register_object_path(
 		      d->connection,
 		      d->object_path,
 		      &vtable,
-		      d))) {
+		      d,
+		      error))) {
+		jack_error("cannot register object path \"%s\": %s", d->object_path, error->message);
 		r = -ENOMEM;
 		goto fail;
 	}
@@ -526,6 +569,7 @@ success:
 		    filter_handler,
 		    d,
 		    NULL)) {
+		dbus_set_error(error, RESERVE_ERROR_NO_MEMORY, "Cannot add filter");
 		r = -ENOMEM;
 		goto fail;
 	}

linux/alsa/JackAlsaDriver.cpp

@@ -31,7 +31,6 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/time.h>
-#include <regex.h>
 #include <string.h>
 
 #include "JackAlsaDriver.h"
@@ -172,30 +171,31 @@ int JackAlsaDriver::Detach()
     return JackAudioDriver::Detach();
 }
 
-static char* get_control_device_name(const char * device_name)
+extern "C" char* get_control_device_name(const char * device_name)
 {
     char * ctl_name;
-    regex_t expression;
+    const char * comma;
 
-    regcomp(&expression, "(plug)?hw:[0-9](,[0-9])?", REG_ICASE | REG_EXTENDED);
+    /* the user wants a hw or plughw device, the ctl name
+     * should be hw:x where x is the card identification.
+     * We skip the subdevice suffix that starts with comma */
 
-    if (!regexec(&expression, device_name, 0, NULL, 0)) {
-        /* the user wants a hw or plughw device, the ctl name
-         * should be hw:x where x is the card number */
-
-        char tmp[5];
-        strncpy(tmp, strstr(device_name, "hw"), 4);
-        tmp[4] = '\0';
-        jack_info("control device %s",tmp);
-        ctl_name = strdup(tmp);
-    } else {
-        ctl_name = strdup(device_name);
+    if (strncasecmp(device_name, "plughw:", 7) == 0) {
+        /* skip the "plug" prefix" */
+        device_name += 4;
     }
 
-    regfree(&expression);
-
-    if (ctl_name == NULL) {
-        jack_error("strdup(\"%s\") failed.", ctl_name);
+    comma = strchr(device_name, ',');
+    if (comma == NULL) {
+        ctl_name = strdup(device_name);
+        if (ctl_name == NULL) {
+            jack_error("strdup(\"%s\") failed.", device_name);
+        }
+    } else {
+        ctl_name = strndup(device_name, comma - device_name);
+        if (ctl_name == NULL) {
+            jack_error("strndup(\"%s\", %u) failed.", device_name, (unsigned int)(comma - device_name));
+        }
     }
 
     return ctl_name;
@@ -278,16 +278,22 @@ int JackAlsaDriver::Open(jack_nframes_t nframes,
         int playback_card = card_to_num(playback_driver_name);
         char audio_name[32];
 
-        snprintf(audio_name, sizeof(audio_name), "Audio%d", capture_card);
-        if (!JackServerGlobals::on_device_acquire(audio_name)) {
-            jack_error("Audio device %s cannot be acquired...", capture_driver_name);
-            return -1;
+        if (capture_card >= 0) {
+            snprintf(audio_name, sizeof(audio_name), "Audio%d", capture_card);
+            if (!JackServerGlobals::on_device_acquire(audio_name)) {
+                jack_error("Audio device %s cannot be acquired...", capture_driver_name);
+                return -1;
+            }
         }
 
-        if (playback_card != capture_card) {
+        if (playback_card >= 0 && playback_card != capture_card) {
             snprintf(audio_name, sizeof(audio_name), "Audio%d", playback_card);
             if (!JackServerGlobals::on_device_acquire(audio_name)) {
                 jack_error("Audio device %s cannot be acquired...", playback_driver_name);
+                if (capture_card >= 0) {
+                    snprintf(audio_name, sizeof(audio_name), "Audio%d", capture_card);
+                    JackServerGlobals::on_device_release(audio_name);
+                }
                 return -1;
             }
         }

linux/alsa/JackAlsaDriver.h

@@ -39,15 +39,13 @@ class JackAlsaDriver : public JackAudioDriver
     private:
 
         jack_driver_t* fDriver;
-        int fReservedCaptureDevice;
-        int fReservedPlaybackDevice;
 
         void UpdateLatencies();
 
     public:
 
         JackAlsaDriver(const char* name, const char* alias, JackLockedEngine* engine, JackSynchro* table)
-		: JackAudioDriver(name, alias, engine, table),fDriver(NULL),fReservedCaptureDevice(-1),fReservedPlaybackDevice(-1)
+		: JackAudioDriver(name, alias, engine, table),fDriver(NULL)
         {}
         virtual ~JackAlsaDriver()
         {}

linux/alsa/alsa_driver.c

@@ -32,7 +32,6 @@
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/time.h>
-#include <regex.h>
 #include <string.h>
 
 #include "alsa_driver.h"
@@ -137,30 +136,18 @@ alsa_driver_check_capabilities (alsa_driver_t *driver)
 	return 0;
 }
 
+char* get_control_device_name(const char * device_name);
+
 static int
 alsa_driver_check_card_type (alsa_driver_t *driver)
 {
 	int err;
 	snd_ctl_card_info_t *card_info;
 	char * ctl_name;
-	regex_t expression;
 
 	snd_ctl_card_info_alloca (&card_info);
 
-	regcomp(&expression,"(plug)?hw:[0-9](,[0-9])?",REG_ICASE|REG_EXTENDED);
-
-       	if (!regexec(&expression,driver->alsa_name_playback,0,NULL,0)) {
-		/* the user wants a hw or plughw device, the ctl name
-		 * should be hw:x where x is the card number */
-
-		char tmp[5];
-		strncpy(tmp,strcasestr(driver->alsa_name_playback,"hw"),4);
-		tmp[4]='\0';
-		jack_info("control device %s",tmp);
-		ctl_name = strdup(tmp);
-	} else {
-		ctl_name = strdup(driver->alsa_name_playback);
-	}
+	ctl_name = get_control_device_name(driver->alsa_name_playback);
 
 	// XXX: I don't know the "right" way to do this. Which to use
 	// driver->alsa_name_playback or driver->alsa_name_capture.
@@ -175,7 +162,6 @@ alsa_driver_check_card_type (alsa_driver_t *driver)
 
 	driver->alsa_driver = strdup(snd_ctl_card_info_get_driver (card_info));
 
-	regfree(&expression);
 	free(ctl_name);
 
 	return alsa_driver_check_capabilities (driver);