Bug 32639

Freeamp has a preference to cause it to delegate actions to an already
running freeamp and exit. This does not, and *cannot*, work on FreeBSD the
way it is designed.

I'm going to describe a mismatch between freeamp's design and FreeBSD's
process implementation.

The first instance of freeamp to start up creates a semaphore. It puts its
PID in the semaphore using semctl. Other freeamps check the value in the
semaphore to see if they are subordinate to the first and should exit if the
user pref AllowMultipleInstances is false.

Semctl uses a union semun to hold values. This union is just a bit misleading.
It looks like this (/usr/include/sys/sem.h):

union semun {
	int	val;		/* value for SETVAL */
	struct	semid_ds *buf;	/* buffer for IPC_STAT & IPC_SET */
	u_short	*array;		/* array for GETALL & SETALL */
};

You'd think you could put an int value into a semaphore with the SETVAL
operator, wouldn't you? Yes? You'd be wrong. The value a semaphore can hold
is really a "u_short", seen in the array for GETALL and SETALL values. 
(Is this clearly documented anywhere? Well, no, but I don't recall ever
seeing it clearly documented anywhere outside of a Stevens book.)

FreeBSD uses an int as its PID type. A 32 bit int. It's defined in
<machine/ansi.h>, which is included by <sys/inttypes.h>, which is included
by <sys/types.h>, which is included by <unistd.h>. Easy to find.

C++ does type checking. So, we'll get warned when we try to store a 32-bit
PID into a 16-bit semaphore value, right? Umm, no, that brain-damaged union
with the "int val" that really only holds a short int kills our hopes of
that.

So, can this be fixed? No, not short of a design change in freeamp to
communicate that PID some other way to other, non-child freeamp processes.

Fix: 

Design a new method for freeamp to use to communicate the PID of the
"master" instance to any new new freeamps that are started.

For Extra Credit:

Try to get the freeamp team, who have never answered email in my experience,
to accept a design-changing patch for an OS they don't officially support.

For Extra Extra Credit:

Try to get the freeamp team to admit to having brain-lock when they assumed
that a PID was a short int.

For the Nobel Prize:

Fix the "union semun" to have a "short int" val, or fix the semaphore
implementation to allow "int" values. Fix all the applications and ports
that break because of the fix; remember, things will break because of
signedness as well as number of bits. Convince at least 2 authors whose apps
you broke that *you* aren't brain-damaged for changing a broken legacy
structure that so many things depend on.
How-To-Repeat: 
Set the AllowMultipleInstances preference to false. Start up two Freeamp
instances. See two Freeamp instances.

Note: if the master freeamp has a low process ID (< USHRT_MAX), the feature
should work. If this happens, start and kill a few thousand processes and
try again. 

You can also patch freeamp/base/unix/src/bootstrap.cpp to show you what PID
it thinks is its master. I can supply that patch on request.

To see the semaphore problem in an isolated state, compile and run this
program:

---8<-snip---8<-snip---8<-snip---8<-snip---8<-snip---8<-snip---8<---
#include <stdio.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/ipc.h>
#include <sys/sem.h> 
#include <sys/shm.h> 
#include <sys/types.h>
#include <signal.h>

const int iSemKey = 0xDEADBEEF;

int 
main(int argc, char **argv) 
{
    int        rc = 0;
    int        iCmdSem = -1;
    key_t      tSemKey = iSemKey;
    int        iSemVal;

    union semun unsem;

    iCmdSem = semget(tSemKey, 1, IPC_CREAT | 0660);
    unsem.val = 0xDEADBEEF;
    semctl(iCmdSem, 0, SETVAL, unsem);
    iSemVal = semctl(iCmdSem, 0, GETVAL, unsem);
    if (iSemVal == unsem.val) {
      printf("semctl(SETVAL) and GETVAL agree\n");
    } else {
      rc = 1;
      printf("OHSHIT! semctl(SETVAL,%08x) but GETVAL=> %08x\n",
	     unsem.val, iSemVal);
    }

    return rc;
}
---8<-snip---8<-snip---8<-snip---8<-snip---8<-snip---8<-snip---8<---