0

I have a client server project using semaphores. I run both from the same folder, and they use the same key. Now, I want the server to lock the semaphore, so Client can't run commands until server frees it, but the client ignores the server's lock. I do not understand where my mistake is. Server code:

#include<stdio.h>
#include<string.h>
#include<pthread.h>
#include<stdlib.h>
#include<unistd.h>
#include <sys/types.h>
#include <sys/ipc.h>
#include <sys/sem.h>
#include <signal.h>
#include <sys/shm.h>
#include <sys/stat.h>
#define FLAGS IPC_CREAT | 0644
union semun {
    int val;
    struct semid_ds *buf;
    ushort *array; };
union semun semarg;
struct sembuf sops[1];
int main() {
    semarg.val=1;
    int resultsCreator=open("results.txt",O_CREAT|O_RDWR);
    key_t key;
    key = ftok("results.txt", 'k');
    int shmid = shmget(key, 12, FLAGS);
    int semfor = semget(key, 1, IPC_CREAT | IPC_EXCL | 0666);
    semctl ( semfor , 0 , SETVAL , semarg );
    sops->sem_num = 0;
    sops->sem_flg = 0;
    sops->sem_op = -1;
    int k = semop ( semfor , sops , 1 ); //lock the semaphore
    char* shmaddr;
    int numWaiting =0;
    while(1){   
        sleep(2); //CHECK EVERY 2 SECONDS IF SOMEONE IS WAITING
        numWaiting = semctl(semfor, 0, GETNCNT, semarg); 
        if(numWaiting==0){  
            printf("none waiting\n");
            continue; }
        printf("more than one waiter\n");  //NEVER GETS HERE
    } //END WHILE

Client code:

#include<stdio.h>
#include<string.h>
#include<pthread.h>
#include<stdlib.h>
#include<unistd.h>
#include <sys/types.h>
#include <sys/ipc.h>
#include <sys/sem.h>
#include <signal.h>
#include <sys/shm.h>
#include <sys/stat.h>
#define FLAGS IPC_CREAT | 0644
union semun {
    int val;
    struct semid_ds *buf;
    ushort *array;
};   
union semun semarg;
struct sembuf sops[1];
int main()
{
    key_t key;
    key = ftok("results.txt", 'k');
    int shmid = shmget(key, 12, FLAGS);
    semarg.val=1;
    int semfor = semget(key, 0, 0666);
    semctl ( semfor , 0 , SETVAL , semarg );
    sops->sem_num = 0;
    sops->sem_flg = 0;
    sops->sem_op = -1;
    semop ( semfor , sops , 1 );
    printf("skipped lock\n"); //PRINTS IT, EVEN WHEN IT'S STILL LOCKED
    sops->sem_op = 1;
    semop ( semfor , sops , 1 );
    return 0;
}

why does the client ignore the server's semaphore lock?

a3f
  • 8,517
  • 1
  • 41
  • 46
Zohar Argov
  • 143
  • 1
  • 1
  • 11
  • Does the real code also miss any error checking? – alk May 31 '16 at 08:55
  • /i deleted some non-important part to make it as short as possible, but my code has no errors, I showed here exactly what it prints – Zohar Argov May 31 '16 at 08:58
  • 1
    *i deleted some non-important part* If you don't know why it's not working, you can't know what parts are non-important. Print the return value of `semget()` - every time you run it. – Andrew Henle May 31 '16 at 10:59
  • I have this check for every system-call, and i deleted it since it never points to an error (never returns -1 ) – Zohar Argov May 31 '16 at 11:28
  • leave all error checking in your code, have it only output a message to `stderr` when an error occurs. – user3629249 May 31 '16 at 22:33
  • the 'server' code: 1) is missing the header that defines `O_CREAT` and `O_RDWR` The 'main()` function is missing one or more lines at the end. 3) there is code for `shared memory mapping`, but it is not done properly. – user3629249 May 31 '16 at 22:39
  • the `client` code: 1) there is code for shared memory mapping, but it is not done properly. – user3629249 May 31 '16 at 22:44
  • this line: `int semfor = semget(key, 0, 0666);`, because the second parameter is 0, will not create any semaphores. Suggest changing that 0 to 1 – user3629249 May 31 '16 at 22:45
  • regarding this line: `semctl ( semfor , 0 , SETVAL , semarg );`, if your machine is a linux box, then the `union semun` is missing a forth entry, a `struct seminfo *_buf;` – user3629249 May 31 '16 at 22:49
  • normally, when discussing semaphores, the general names for the executables are `producer` and `consumer`. How is that relating to the general names: `client` and `server`? – user3629249 May 31 '16 at 22:51
  • the array `sops[]` when referenced in lines like: `sops->sem_num = 0;` should be written as: `sops[0]->sem_num = 0;` – user3629249 May 31 '16 at 22:56
  • for ease of readability and understanding, 1) separate code blocks (for, if, else, while, do...while, switch, case, default) via a blank line. 2) follow the axiom: *only one statement per line and (at most) one variable declaration per statement.* – user3629249 May 31 '16 at 22:58
  • the posted code for `server` is missing this line: `#include ` – user3629249 May 31 '16 at 22:59
  • the `server` code is opening a file: `results.txt` for read/write, but never uses the resulting file descriptor. – user3629249 May 31 '16 at 23:02
  • when compiling, always enable all the warnings, then fix those warnings. (for `gcc`, at a minimum use: `-Wall -Wextra -pedantic` I also use: `-Wconversion -std=gnu99`) – user3629249 May 31 '16 at 23:03

1 Answers1

0

There was a similar question to which I answered so I'll just copy it over here

If you're going to be technical, if you're syncing tasks between threads you should use Semaphore. Example reading input before parsing it. Here's an answer on semaphores.

But if you're using shared resources, and need to avoid race condition/two threads accesing at the same time, you should use mutexes. Here's a question on what is a mutex.

Also look at the disambiguation by Michael Barr which is a really good.

I would read both question thoroughly and the disambiguation, and you might actually end up not using semaphore and just mutexes since from what you explained you're only controlling a shared resource.

Common semaphore function

int sem_init(sem_t *sem, int pshared, unsigned int value); //Use pshared with 0, starts the semaphore with a given value

int sem_wait(sem_t *sem);//decreases the value of a semaphore, if it's in 0 it waits until it's increased

int sem_post(sem_t *sem);//increases the semaphore by 1

int sem_getvalue(sem_t *sem, int *valp);// returns in valp the value of the semaphore the returned int is error control

int sem_destroy(sem_t *sem);//destroys a semaphore created with sim_init

Common Mutexes functions (for unix)

int pthread_mutex_init(pthread_mutex_t *p_mutex, const pthread_mutexattr_t *attr); //starts mutex pointed by p_mutex, use attr NULL for simple use

int pthread_mutex_lock(pthread_mutex_t *p_mutex); //locks the mutex

int pthread_mutex_unlock(pthread_mutex_t *p_mutex); //unlocks the mutex

int pthread_mutex_destroy(pthread_mutex_t *p_mutex);//destroys the mutex

I'm not sure why your code doesn't work but from what you're explaining you needn't use a semaphore, since there's no synchronization between the client and the server only the use of a shared resource. Try re-writing the semaphore part with a mutex, you'll see it will seem much more natural and will probably end up fixing the problem.

Community
  • 1
  • 1
Mr. Branch
  • 442
  • 2
  • 13