Fixed the final semaphore stuff

assignment2_handout.c

@@ -57,7 +57,7 @@
 #define true 1
 #endif
 // Print the number of people in the lifts & on the floors
-//#define POPULATIONS
+#define POPULATIONS
 
 // --------------------------------------------------
 // Information about a floor in the building
@@ -82,6 +82,7 @@ typedef struct
 	int peopleinlift;			// The number of people in the lift
 	int stops[NFLOORS];			// How many people are going to each floor
 	semaphore stopsem[NFLOORS]; // People in the lift wait on one of these
+	semaphore addToStop;
 } lift_info;
 
 // --------------------------------------------------
@@ -111,7 +112,7 @@ void print_at_xy_fast(int x, int y, const char *s)
 	gotoxy(x, y);
 
 	// Slow things down
-	Sleep(0);
+	//Sleep(0);
 
 	// Print the string
 	printf("%s", s);
@@ -135,7 +136,7 @@ void print_at_xy(int x, int y, const char *s)
 	gotoxy(x, y);
 
 	// Slow things down
-	Sleep(1);
+	//Sleep(1);
 
 	// Print the string
 	printf("%s", s);
@@ -205,6 +206,8 @@ void get_into_lift(lift_info *lift, int direction)
 			// Wait til we can be the lift that is loading
 			semaphore_wait(&targetLiftSelect[lift->position]);
 			targetLift[lift->position] = lift;
+			// lock the addtostop
+			semaphore_wait(&lift->addToStop);
 			semaphore_signal(s);
 		}
 		else
@@ -231,7 +234,7 @@ void *lift_thread(void *p)
 	lift.direction = UP;   // Lift starts going up
 	lift.peopleinlift = 0; // Lift starts empty
 	// lift is ok with loading a single person
-	//semaphore_create(&lift.loading, 1);
+	semaphore_create(&lift.addToStop, 1);
 	//semaphore_create(&lift.buttonPress, 1);
 
 	for (i = 0; i < NFLOORS; i++)
@@ -265,12 +268,14 @@ void *lift_thread(void *p)
 		// Drop off passengers on this floor
 		while (lift.stops[lift.position] != 0)
 		{
+			// These are sequential, so don't need protecting
 			// One less passenger in lift
+			
 			lift.peopleinlift--;
 
 			// One less waiting to get off at this floor
 			lift.stops[lift.position]--;
-
+			
 			// Wait for exit lift delay
 			Sleep(GETOUTSPEED);
 
@@ -302,7 +307,11 @@ void *lift_thread(void *p)
 		print_at_xy(no * 4 + 1, NFLOORS - lift.position, (lift.direction + 1 ? " " : lc));
 
 		// Move lift
+		// Hold the lift while someone is being added, before moving off
+		// this really could go anywhere, but it fits the metaphor to wrap it here (doesn't need to wrap anything)
+		semaphore_wait(&lift.addToStop);
 		lift.position += lift.direction;
+		semaphore_signal(&lift.addToStop);
 
 		// Check if lift is at top or bottom
 		if (lift.position == 0 || lift.position == NFLOORS - 1)
@@ -364,17 +373,24 @@ void *person_thread(void *p)
 			// Wait for a lift to arrive (going down)
 		}
 		// We've pressed our button
-
+		// waiting for a lift's doors to open
 		semaphore_wait(s);
-
-		// Which lift we are getting into
+		// Which lift we are getting into. The targetLift has locked the variable, so once we have our reference,
 		lift = targetLift[from];
-		// Realease the target-lift semaphore
+		// Realease the targetLift ASAP so another lift can load someone
 		semaphore_signal(&targetLiftSelect[lift->position]);
-		// Add one to passengers waiting for floor
-		lift->stops[to]++;
+		// GetIntoLift() continues its loop, from here (this passenger is signalled to enter; so we definitely need to hold a lock around
+		// this value)
+		// When GetIntoLift() decides that a passenger should enter, it obtains a lock of addToStop. 
+		// We release that lock here. Finally, once all passengers have been picked up and the lift decides to move on, if this lock is
+		// still held, then the lift wont change floor (protecting against unloading issues)
 
+		// We could do this by extending the targetLiftSelect semaphore through the same condition, but that would affect the performance of all lifts on this floor
+		lift->stops[to]++;
+		semaphore_signal(&lift->addToStop);
 		// Press button if we are the first
+		// Technically, the semaphore should extend through this conditional, but as it is only a read operation whose state is should to
+		// be greater than 0 here, it doesn't significantly matter to overwrite the char again (the performance impact is low)
 		if (lift->stops[to] == 1)
 		{
 			// Print light for destination
@@ -470,7 +486,7 @@ void *update_floor_counts(void *p)
 {
 	int i;
 	int k;
-	char str2[17] = {';',' ', 'i', 't', 'e', 'r', 's', ' ', 's', 'i', 'n', 'c', 'e', ' ', '0', ':', ' '};
+	char str2[18] = {';',' ','\t', 'i', 't', 'e', 'r', 's', ' ', 's', 'i', 'n', 'c', 'e', ' ', '0', ':', ' '};
 	while (true)
 	{
 		for (i = -1; i < NFLOORS; i++)
@@ -489,7 +505,7 @@ void *update_floor_counts(void *p)
 				sprintf(str3, "%d", floorCountZeroInc[i]);
 				strcat(str, str2);
 				strcat(str, str3);
-				print_at_xy_fast(4 * NLIFTS + NPEOPLE + 5, NFLOORS - i, "                                 ");
+				print_at_xy_fast(4 * NLIFTS + NPEOPLE + 5, NFLOORS - i, "                                          ");
 				print_at_xy_fast(4 * NLIFTS + NPEOPLE + 5, NFLOORS - i, str);
 			}
 			else