The following code should simulate a port with M places and N ships accessing to it. The problem: in the "entering_request" function, the program controls how many ships are in port, if there are too many ships the program should block on a condition variable. The problem is that it does not work, and I usually get more than M ships at runtime. I think that I am misunderstanding something, anybody could enlighten me? #include <pthread.h> #include <stdio.h> #include <stdlib.h> #include <time.h> #define N 10 #define M 5 struct port { pthread_mutex_t mutex; pthread_cond_t full; int ships; int waiting; } Port = { PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER, 0, 0 }; void* ship (void* arg); int main() { void* nothing; pthread_t ships[N]; int i; pthread_setconcurrency(N); for (i=0; i<N; i++) pthread_create (&ships[i], NULL, ship, NULL); for (i=0; i<N; i++) pthread_join (ships[i], ¬hing); printf("The end\n"); return 0; } void entering_request() { pthread_mutex_lock(&Port.mutex); if (Port.ships>=M) { Port.waiting++; printf("ship %d blocked\n", pthread_self()); pthread_cond_wait(&Port.full, &Port.mutex); printf("ship %d unblocked\n", pthread_self()); Port.waiting--; } } void entering() { printf ("ship %d enters the port", pthread_self()); printf ("\n%d ships in port\n", Port.ships); } void leaving_entrance() { Port.ships++; printf("Ships inside the port: %d\n", Port.ships); pthread_mutex_unlock(&Port.mutex); } void stationing () { int l, i; srand(time(NULL)); printf("La ship %d is inside with other %d ships.\n", pthread_self(), Port.ships); l = rand(); /* Wasting time */ for (i=0; i<l; i++); } void exit_request() { pthread_mutex_lock(&Port.mutex); } void exiting() { printf ("ship %d out of the port\n", pthread_self()); printf ("%d ships\n remain in port", Port.ships); } void leaving_exit() { Port.ships--; if (Port.waiting) pthread_cond_signal (&Port.full); pthread_mutex_unlock(&Port.mutex); } void* ship (void* arg) { int i; for (i=0; i<3000; i++) { entering_request(); entering(); leaving_entrance(); stationing(); exit_request(); exiting(); leaving_exit(); } return NULL; }
First, pthread_setconcurrency() does nothing on Linux. Second of all, the lack of a Makefile made it difficult for me to run this program. Third of all, a documentation issue: the use of M and N is appropriate for a mathematical formula, but not for software; better would have been the names NBERTHS and NSHIPS. The condition "full" needs to be described. What do you actually intend it to do? The constant 3000 also needs to be described and probably made into an explicit named constant. If you did those things it would be easier for me to look at your code and understand what you're trying to do. The variables "ships" and "waiting" might be better named "ndocked_ships" and "nwaiting_ships", or something of the sort. Incidentally, document that you use waiting purely for printing debugging messages. It would also be very nice if your code would compile cleanly under GCC with various warnings enabled. I personally have the environment variable CFLAGS set to the following by default: export CFLAGS="-pipe -ggdb3 -W -Wall -Wbad-function-cast -Wcast-align -Wpointer-arith -Wcast-qual -Wshadow -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -fkeep-static-consts -fkeep-inline-functions -Wundef -Wwrite-strings -Wno-aggregate-return -Wmissing-noreturn -Wnested-externs -Wtrigraphs -Wconversion -Wsign-compare -Wno-float-equal -Wmissing-format-attribute -Wno-unreachable-code -Wdisabled-optimization -Wendif-labels" Now to the meat of the matter, the *pthread* calls: I have personally always used pthread_cond_wait() from inside of a while() loop, just in case the condition status has changed between the time someone sent the signal and the time the waiting thread received it. This code would be more robust and simpler if you assumed that the condition being signalled did not necessarily mean that there was actually a berth available, just that there might be one. Then, at least while you're debugging, use pthread_cond_broadcast() instead of pthread_cond_signal(). And get rid of the if() statement guarding the pthread_cond_signal (now to be pthread_cond_broadcast()). Just always broadcast; if there are no listeners, the runtime system will throw away the broadcast. You can add the if() back after you have your implementation working in the simple mode. E, finalmente, chi é Tazio? --Steve Augart -- Steven Augart Jikes RVM, an open source Java Virtual Machine http://oss.software.ibm.com/jikesrvm Office: +1 914/784-6743 T.J. Watson Research Center, IBM
Alle 21:34, mercoledì 28 gennaio 2004, Steven Augart ha scritto:
First, pthread_setconcurrency() does nothing on Linux.
I know, but it is an exercise and it was required (not targetted specifically to linux).
Second of all, the lack of a Makefile made it difficult for me to run this program.
gcc -lpthread file.c -o yourchoice
Third of all, a documentation issue: the use of M and N is appropriate for a mathematical formula, but not for software; better would have been the names NBERTHS and NSHIPS.
Right, but I did not choose those names. The other names were roughly translated into something meaningful in english, maybe not always the best choice everytime. Uhm; I am sure I did not take a good choice almost everything, but the italian names (chosen by the teacher)
The condition "full" needs to be described. What do you actually intend it to do?
When the port if full, you can not place your ship there.
The constant 3000 also needs to be described and probably made into an explicit named constant.
That means nothing, just a way to make the "ships" do something many times.
If you did those things it would be easier for me to look at your code and understand what you're trying to do.
The variables "ships" and "waiting" might be better named "ndocked_ships" and "nwaiting_ships", or something of the sort. Incidentally, document that you use waiting purely for printing debugging messages.
it's used also in an "if" statement. Btw this is surely not a "real world" program. Thank you for your advices anyway:) Right.
It would also be very nice if your code would compile cleanly under GCC with various warnings enabled. I personally have the environment variable CFLAGS set to the following by default: export CFLAGS="-pipe -ggdb3 -W -Wall -Wbad-function-cast -Wcast-align -Wpointer-arith -Wcast-qual -Wshadow -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -fkeep-static-consts -fkeep-inline-functions -Wundef -Wwrite-strings -Wno-aggregate-return -Wmissing-noreturn -Wnested-externs -Wtrigraphs -Wconversion -Wsign-compare -Wno-float-equal -Wmissing-format-attribute -Wno-unreachable-code -Wdisabled-optimization -Wendif-labels"
Now to the meat of the matter, the *pthread* calls:
I have personally always used pthread_cond_wait() from inside of a while() loop, just in case the condition status has changed between the time someone sent the signal and the time the waiting thread received it. This code would be more robust and simpler if you assumed that the condition being signalled did not necessarily mean that there was actually a berth available, just that there might be one. Then, at least while you're debugging, use pthread_cond_broadcast() instead of pthread_cond_signal(). And get rid of the if() statement guarding the pthread_cond_signal (now to be pthread_cond_broadcast()). Just always broadcast; if there are no listeners, the runtime system will throw away the broadcast. You can add the if() back after you have your implementation working in the simple mode.
Yes I figured it out while I was out to eat. A really silly problem after all. Strangely the examples from my teacher do not use always a while statement.
E, finalmente, chi é Tazio?
That's me. Why? I am not praising myself "praise" is a nickname given to me by some guys who did not know the english meaning of that word.
--Steve Augart
Ma tu sai l'italiano? Praise
Your replies make a lot of sense. And I appreciate your good-natured acceptance of my criticism of the program. praisetazio wrote:
Alle 21:34, mercoledì 28 gennaio 2004, Steven Augart ha scritto:
Incidentally, document that you use waiting purely for printing debugging messages.
it's used also in an "if" statement.
Oops, you're right. I was already thinking of your program partly as I would have written it, where the pthread_cond_signal() would always happen, not be inside the if() statement.
Now to the meat of the matter, the *pthread* calls:
I have personally always used pthread_cond_wait() from inside of a while() loop, just in case the condition status has changed between the time someone sent the signal and the time the waiting thread received it. This code would be more robust and simpler if you assumed that the condition being signalled did not necessarily mean that there was actually a berth available, just that there might be one. Then, at least while you're debugging, use pthread_cond_broadcast() instead of pthread_cond_signal(). And get rid of the if() statement guarding the pthread_cond_signal (now to be pthread_cond_broadcast()). Just always broadcast; if there are no listeners, the runtime system will throw away the broadcast. You can add the if() back after you have your implementation working in the simple mode.
Yes I figured it out while I was out to eat. A really silly problem after all. Strangely the examples from my teacher do not use always a while statement.
It may be possible to do without the while() statement if you've thought through all the possible interactions. Maybe your teacher is a more thoughtful programmer than I am. It also may be that your teacher is only human and makes mistakes too. In production coding, I use these things as part of writing robust code.
Ma tu sai l'italiano?
Praise
Sì. La mia mamma é abruzzese; é immigrata agli Stati Uniti e dopo si é sposata a un bavarese. Posso comprare Oggi, Gente, La Repubblica, e altre pubblicazioni in edicola (per il meno, nelle tre zone metropolitane in cui ho abitato, quelle di Los Angeles, Boston, e Nuova York.) Mi abbono a "America Oggi" e all'edizione italiana di "Oggi." --Stefano (Steve) -- Steven Augart Jikes RVM, an open source Java Virtual Machine http://oss.software.ibm.com/jikesrvm Office: +1 914/784-6743 T.J. Watson Research Center, IBM
HI It is working for me... (SUSE 9.0, gcc (GCC) 3.3.1 (SuSE Linux)) I always get 5 ships in the port. Look at the start of the program five ships enters to the port and five are blocked. Zsolt praisetazio wrote:
The following code should simulate a port with M places and N ships accessing to it. The problem: in the "entering_request" function, the program controls how many ships are in port, if there are too many ships the program should block on a condition variable. The problem is that it does not work, and I usually get more than M ships at runtime. I think that I am misunderstanding something, anybody could enlighten me?
#include <pthread.h> #include <stdio.h> #include <stdlib.h> #include <time.h>
#define N 10 #define M 5
struct port { pthread_mutex_t mutex; pthread_cond_t full; int ships; int waiting; } Port = { PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER, 0, 0 };
void* ship (void* arg);
int main() { void* nothing; pthread_t ships[N]; int i; pthread_setconcurrency(N); for (i=0; i<N; i++) pthread_create (&ships[i], NULL, ship, NULL); for (i=0; i<N; i++) pthread_join (ships[i], ¬hing); printf("The end\n"); return 0; }
void entering_request() { pthread_mutex_lock(&Port.mutex); if (Port.ships>=M) { Port.waiting++; printf("ship %d blocked\n", pthread_self()); pthread_cond_wait(&Port.full, &Port.mutex); printf("ship %d unblocked\n", pthread_self()); Port.waiting--; } }
void entering() { printf ("ship %d enters the port", pthread_self()); printf ("\n%d ships in port\n", Port.ships); }
void leaving_entrance() { Port.ships++; printf("Ships inside the port: %d\n", Port.ships); pthread_mutex_unlock(&Port.mutex); }
void stationing () { int l, i; srand(time(NULL)); printf("La ship %d is inside with other %d ships.\n", pthread_self(), Port.ships); l = rand(); /* Wasting time */ for (i=0; i<l; i++); }
void exit_request() { pthread_mutex_lock(&Port.mutex); }
void exiting() { printf ("ship %d out of the port\n", pthread_self()); printf ("%d ships\n remain in port", Port.ships); }
void leaving_exit() { Port.ships--; if (Port.waiting) pthread_cond_signal (&Port.full); pthread_mutex_unlock(&Port.mutex); }
void* ship (void* arg) { int i; for (i=0; i<3000; i++) { entering_request(); entering(); leaving_entrance(); stationing(); exit_request(); exiting(); leaving_exit(); } return NULL; }
It's not surprising that it worked for you perfectly. Thread-based programs are pretty sensitive to minor changes in scheduling and environment. If they're written robustly, then they'll do well on all, but when there are subtle bugs it's pretty common for them to do fine on version m.n of an implementation and then to fail on version m.n+1. --Steve Augart Lukacs Zsolt wrote:
HI
It is working for me... (SUSE 9.0, gcc (GCC) 3.3.1 (SuSE Linux))
-- Steven Augart Jikes RVM, an open source Java Virtual Machine http://oss.software.ibm.com/jikesrvm Office: +1 914/784-6743 T.J. Watson Research Center, IBM
participants (3)
-
Lukacs Zsolt -
praisetazio -
Steven Augart