"The usage of semaphores is subtly wrong"
- by Hoonose
This past semester I was taking an OS practicum in C, in which the first project involved making a threads package, then writing a multiple producer-consumer program to demonstrate the functionality. However, after getting grading feedback, I lost points for "The usage of semaphores is subtly wrong" and "The program assumes preemption (e.g. uses yield to change control)" (We started with a non-preemptive threads package then added preemption later. Note that the comment and example contradict each other. I believe it doesn't assume either, and would work in both environments).
This has been bugging me for a long time - the course staff was kind of overwhelmed, so I couldn't ask them what's wrong with this over the semester. I've spent a long time thinking about this and I can't see the issues. If anyone could take a look and point out the error, or reassure me that there actually isn't a problem, I'd really appreciate it.
I believe the syntax should be pretty standard in terms of the thread package functions (minithreads and semaphores), but let me know if anything is confusing.
#include <stdio.h>
#include <stdlib.h>
#include "minithread.h"
#include "synch.h"
#define BUFFER_SIZE 16
#define MAXCOUNT 100
int buffer[BUFFER_SIZE];
int size, head, tail;
int count = 1;
int out = 0;
int toadd = 0;
int toremove = 0;
semaphore_t empty;
semaphore_t full;
semaphore_t count_lock; // Semaphore to keep a lock on the
// global variables for maintaining the counts
/* Method to handle the working of a student
* The ID of a student is the corresponding minithread_id */
int student(int total_burgers) {
int n, i;
semaphore_P(count_lock);
while ((out+toremove) < arg) {
n = genintrand(BUFFER_SIZE);
n = (n <= total_burgers - (out + toremove)) ? n : total_burgers - (out + toremove);
printf("Student %d wants to get %d burgers ...\n", minithread_id(), n);
toremove += n;
semaphore_V(count_lock);
for (i=0; i<n; i++) {
semaphore_P(empty);
out = buffer[tail];
printf("Student %d is taking burger %d.\n", minithread_id(), out);
tail = (tail + 1) % BUFFER_SIZE;
size--;
toremove--;
semaphore_V(full);
}
semaphore_P(count_lock);
}
semaphore_V(count_lock);
printf("Student %d is done.\n", minithread_id());
return 0;
}
/* Method to handle the working of a cook
* The ID of a cook is the corresponding minithread_id */
int cook(int total_burgers) {
int n, i;
printf("Creating Cook %d\n",minithread_id());
semaphore_P(count_lock);
while ((count+toadd) <= arg) {
n = genintrand(BUFFER_SIZE);
n = (n <= total_burgers - (count + toadd) + 1) ? n : total_burgers - (count + toadd) + 1;
printf("Cook %d wants to put %d burgers into the burger stack ...\n", minithread_id(),n);
toadd += n;
semaphore_V(count_lock);
for (i=0; i<n; i++) {
semaphore_P(full);
printf("Cook %d is putting burger %d into the burger stack.\n", minithread_id(), count);
buffer[head] = count++;
head = (head + 1) % BUFFER_SIZE;
size++;
toadd--;
semaphore_V(empty);
}
semaphore_P(count_lock);
}
semaphore_V(count_lock);
printf("Cook %d is done.\n", minithread_id());
return 0;
}
/* Method to create our multiple producers and consumers
* and start their respective threads by fork */
void starter(int* c){
int i;
for (i=0;i<c[2];i++){
minithread_fork(cook, c[0]);
}
for (i=0;i<c[1];i++){
minithread_fork(student, c[0]);
}
}
/* The arguments are passed as command line parameters
* argv[1] is the no of students
* argv[2] is the no of cooks */
void main(int argc, char *argv[]) {
int pass_args[3];
pass_args[0] = MAXCOUNT;
pass_args[1] = atoi(argv[1]);
pass_args[2] = atoi(argv[2]);
size = head = tail = 0;
empty = semaphore_create();
semaphore_initialize(empty, 0);
full = semaphore_create();
semaphore_initialize(full, BUFFER_SIZE);
count_lock = semaphore_create();
semaphore_initialize(count_lock, 1);
minithread_system_initialize(starter, pass_args);
}
