1

For some reason, flock(fd, LOCK_UN) returns 0 and the file is still locked from the second iteration on. So the first time, the file gets unlocked by the write section and the read section can lock it successfully. When i = 1, the write section can't unlock the file anymore and the lock of the read section fails.

I edited open() now as follows.

fd = open(filename, O_WRONLY | O_APPEND| O_CREAT, S_IRWXU | S_IRWXG | S_IRWXO);

After I did, every lock and unlock works. Those parameters give read, write and execute permissions to the user. So I am guessing that after the deletion of the file, something goes wrong. I now added perror() to every check and during the second iteration (i = 1), the lock of the read section fails with "Bad file descriptor". Can someone explain this to me?

// Tell the headers to use the POSIX extension
#if __STDC_VERSION__ >= 199901L
#define _XOPEN_SOURCE 600
#else
#define _XOPEN_SOURCE 500
#endif /* __STDC_VERSION__ */

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/file.h>
#include <errno.h>

#define POLL_WAIT_TIME_S 2

const char testString[] = "Test test test\n";

int main() {

    int fd;                                                                     // Create file descriptor variable
    char *buff = (char *) calloc(255, sizeof(char));            // Create buffer for read()
    char* filename = "testfile";                                                // Set filename
    int status;

    while(1) {

        // Create a file and write a text into it
        fd = open(filename, O_WRONLY | O_APPEND| O_CREAT);                          // Create file descriptor
        printf("%d\n", fd);

        status = flock(fd, LOCK_EX);                                                // Lock file using file descriptor
        if(status == 0) 
            printf("\"%s\" locked successfully by write process!\n", filename);
        else {
            printf("\"%s\" not locked successfully by write process!\n", filename);
            perror("Error: ");
        }



        size_t charsWritten = write(fd, testString,  sizeof(testString));                           // Write to file using file descriptor
        printf("%ld characters written to \"%s\" \n", charsWritten, filename);

        if(status == 0) {
            status = flock(fd, LOCK_UN);
            if(status == 0) 
                printf("\"%s\" unlocked by write process!\n", filename);
            else {
                printf("\"%s\" could not be unlocked by write process!\n", filename);
                perror("Error: ");
            }

        }

        close(fd);                                                                  // Close file descriptor

        // Open a file and read its content
        fd = open(filename, O_RDONLY);

        status = flock(fd, LOCK_EX);                                                // Lock file using file descriptor
        if(status == 0) { 
            printf("\"%s\" locked successfully by read process!\n", filename);
            printf("\nReading file content: \n");
            size_t sz = read(fd, buff, (size_t) sizeof(testString));
            buff[sz] = '\0';
            printf("%s", buff);                                                         // Print buffer content to the screen
            printf("\n");
        }
        else {
            printf("\"%s\" not locked successfully by read process!\n", filename);
            perror("Error: ");
        }

        if(status == 0) {
            status = flock(fd, LOCK_UN);
            if(status == 0) 
                printf("\"%s\" unlocked by read process!\n", filename);
            else {
                printf("\"%s\" could not be unlocked by read process!\n", filename);
                perror("Error: ");
            }
        }

        close(fd);

        // Delete the file
        if(remove(filename) == 0)
            printf("\"%s\" deleted successfully!\n", filename);
        else {
            printf("Unable to delete %s\n", filename);
            perror("Error: ");
        }

        printf("\n");
        sleep(POLL_WAIT_TIME_S);
    }   

    free(buff);
    return 0;
}


neolith
  • 699
  • 1
  • 11
  • 20
  • 3
    How do you know the file is still locked? – Shawn May 26 '20 at 14:43
  • The second lock fails and I can see it in the file manager. I edited the question and posted the definitely failing code now. – neolith May 26 '20 at 14:59
  • I'm confused. You say that `flock` returns 0, so what do you mean by "the failed unlock"? – William Pursell May 26 '20 at 15:23
  • Well, flock(fd, LOCK_EX) locks a file (exclusively) and flock(fd, LOCK_UN) unlocks the file. So by "the failed unlock" I mean the latter! – neolith May 26 '20 at 15:24
  • You mention a "read process", but this code contains no fork. What do you mean by "the read process"? – William Pursell May 26 '20 at 15:25
  • Yes, I thought that would come up. That was just for me to know what sections of the code are failing. I changed it to section now. I know it is not a process by defintion. – neolith May 26 '20 at 15:27
  • The code doesn't seem to make sense. The `return(0)` will cause your program to exit and nothing after that point will ever be executed, so why is all that additional code even there? And there is no way the file remains locked after your program exits, that's directly contrary to the documented behavior of `flock`. What makes you think that it is? – Nate Eldredge May 26 '20 at 15:47
  • Can you please explain precisely what happens when you run the program, and why you think that behavior is wrong? Phrases like "the unlock works" are vague and are based on your own interpretation of what you observe, which could be mistaken. Tell us what output you see. – Nate Eldredge May 26 '20 at 15:47
  • 3
    Per your update: indeed, specifying modes is mandatory when you open a file with `O_CREAT`. My suspicion is that the call to `open` actually failed, but your code never checked that. You end up passing file descriptor -1 to `flock`, so of course that fails too. If you properly check the return value of open (as you should with every system call, every time), you will be better able to diagnose such problems. – Nate Eldredge May 26 '20 at 15:55
  • 3
    And another tip: whenever a system call fails, use `perror` to see what error code is returned. This is usually much more informative than just a message saying "failed to do whatever". – Nate Eldredge May 26 '20 at 15:57
  • Well, but why did it not fail the first time then? I don't understand... Thanks for the advice. I see why checking for errors is important now. – neolith May 26 '20 at 15:58
  • I also think that for open() it doesn't work like for fopen(), since open() returns a file descriptor, instead of 0. It returns a small non-negative integer (3 in my case) and checking it with perror() doesn't throw any error at all. – neolith May 26 '20 at 16:05
  • 1
    @neolith — the correct check for `open()` failing is either `if ((fd = open(…)) < 0)` or `if ((fd = open(…)) == -1)`. I use the `< 0` version; there's room to argue that the `== -1` version is sufficient, because `open()` will never return any negative value other than -1 (but all valid file descriptors are non-negative numbers). You pays your money and takes your pick. – Jonathan Leffler May 26 '20 at 16:18
  • 2
    EBADF (Bad file descriptor) means that some function other than `open()` was handed a file descriptor that isn't an open file descriptor — it's bad. Either you miscopied some information or, in this case, got `-1` from `open()` and passed that to `read()` or `write()` or whatever. ALWAYS — but ***ALWAYS*** check whether `open()` calls (and any of its relatives that creates open file descriptors) succeeded. This is a primary place where programs can fail. The fact that you created the file and closed successfully doesn't mean it is still accessible when you try to open it again. It may not be … – Jonathan Leffler May 26 '20 at 16:22
  • @JonathanLeffler: Would this also return information with perror()? Because it doesn't. I also print the file descriptor every time. It is always 3. – neolith May 26 '20 at 16:25
  • 1
    @NateEldredge using `O_CREAT` without a mode doesn't make the `open` fail. It just makes the newly-created file have a mode that's probably not the one you wanted. – Joseph Sible-Reinstate Monica May 26 '20 at 16:25
  • 1
    I have mixed feelings about `perror()` — but it bases its report on the value in `errno`. The `open()` call will set `errno` to an appropriate value and return `-1` to indicate failure. Calling `perror()` prefixes the string you provide to the error message. I use the error reporting functions available in my [SOQ](https://github.com/jleffler/soq) (Stack Overflow Questions) repository on GitHub as files `stderr.c` and `stderr.h` in the [src/libsoq](https://github.com/jleffler/soq/tree/master/src/libsoq) sub-directory. I get what I consider better error messages that way. – Jonathan Leffler May 26 '20 at 16:27
  • 1
    @JonathanLeffler: Right. I think it was created with bogus permissions, perhaps not allowing the owner to write, which meant that when the program was run for the *second* time, the file (which now existed but was not writable) could not be opened. OP didn't check the return value of `open` and therefore misinterpreted this as a failure to lock. My understanding is that it was the second run of the program that failed. – Nate Eldredge May 26 '20 at 16:31
  • 1
    @NateEldredge — yes, the file was created with a quasi-random (indeterminate) access mode; running `ls -l` on the file would be informative. See also [Unix O_CREAT flag without mode specified?](https://stackoverflow.com/q/584210/15168) – Jonathan Leffler May 26 '20 at 16:32
  • @JonathanLeffler: Thanks for the explanation. I think all of this is a little to advanced for me at this point. But I could see that open() returns -1 the seond time. I will just stick to the perimissions now. – neolith May 26 '20 at 16:47

0 Answers0