Linux threads and fopen () fclose () fgets ()

Question

I am looking at some old Linux code that uses pthreads.

In one thread, the file is read through fgets (). The FILE variable is a global variable shared by all threads. (Hey, I didn’t write this ...)

In another thread, from time to time the FILE closes and reopens with a different file name.

For a few seconds after this, the fgets () stream acts as if it continues to read the last record read from the previous file: almost as if there was an error, but fgets () did not return NULL. Then it is sorted and begins reading from a new file.

The code is a bit like this (short for brevity, so I hope it is still clear):

In one thread:

while(gRunState != S_EXIT){ nanosleep(&timer_delay,0); flag = fgets(buff, sizeof(buff), gFile); if (flag != NULL){ // do something with buff... } } 

In another thread:

 fclose(gFile); gFile = fopen(newFileName,"r"); 

There is no lock to ensure that fgets () is not being called at the same time as fclose () / fopen ().

Any thoughts on crash modes that could lead to fgets () error but not return NULL?

Answer 1

How the wrong code is described

The stdio library buffers data, allocating memory to store buffered data. The GNU C library dynamically allocates file structures (some libraries, especially on Solaris, use pointers to statically distributed file structures, but the buffer is still dynamically allocated unless you set up buffering otherwise).

If your thread is working with a copy of the pointer to the global file pointer (since you passed the file pointer to the function as an argument), then it is quite possible that the code will continue to access the data structure that was originally (even if it was freed by closing) and will read data from a buffer that was already present. It will only be when you exit the function or read outside the contents of the buffer that everything starts to go wrong - or the space that was previously allocated for the file structure is redistributed for new use.

 FILE *global_fp; void somefunc(FILE *fp, ...) { ... while (fgets(buffer, sizeof(buffer), fp) != 0) ... } void another_function(...) { ... /* Pass global file pointer by value */ somefunc(global_fp, ...); ... } 

---

Proof of concept code

Tested on MacOS X 10.5.8 (Leopard) with GCC 4.0.1:

 #include <stdio.h> #include <stdlib.h> FILE *global_fp; const char etc_passwd[] = "/etc/passwd"; static void error(const char *fmt, const char *str) { fprintf(stderr, fmt, str); exit(1); } static void abuse(FILE *fp, const char *filename) { char buffer1[1024]; char buffer2[1024]; if (fgets(buffer1, sizeof(buffer1), fp) == 0) error("Failed to read buffer1 from %s\n", filename); printf("buffer1: %s", buffer1); /* Dangerous!!! */ fclose(global_fp); if ((global_fp = fopen(etc_passwd, "r")) == 0) error("Failed to open file %s\n", etc_passwd); if (fgets(buffer2, sizeof(buffer2), fp) == 0) error("Failed to read buffer2 from %s\n", filename); printf("buffer2: %s", buffer2); } int main(int argc, char **argv) { if (argc != 2) error("Usage: %s file\n", argv[0]); if ((global_fp = fopen(argv[1], "r")) == 0) error("Failed to open file %s\n", argv[1]); abuse(global_fp, argv[1]); return(0); } 

When run on its own source code, the output was:

 Osiris JL: ./xx xx.c buffer1: #include <stdio.h> buffer2: ## Osiris JL: 

So, empirical evidence that on some systems the scenario that I have outlined can happen.

How to fix the code

Code correction is well discussed in other answers. If you avoid the problem that I illustrated (for example, avoiding global file pointers), this is easiest. Assuming this is not possible, it might be sufficient to compile with the appropriate flags (on many Unix-like systems, the compiler flag -D_REENTRANT does the job), and you will end up using thread-safe versions of the base standard I / O functions. Otherwise, you may need to establish transparent flow control policies around access to file pointers; mutex or something similar (and modify the code to make sure that threads use the mutex before using the appropriate file pointer).

Answer 2

A FILE * is just a pointer to various resources. If fclose does not reset this resource, it is possible that the values ​​may make sense enough that fgets does not immediately notice it.

However, until you add some locking, I believe that this code is completely broken.

Answer 3

Umm, you really need to control access to the FILE stream with a mutex, at least. You are not looking at the clever implementation of blocking methods; you are looking at really bad (and dusty) code.

Using local FILE streams is the obvious and most elegant fix, just use locks accordingly to ensure that neither of the two streams will work with the same offset of the same file at the same time. Or, more simply put, make sure that the threads block (or do other work) while waiting for the file to lock. For this, it is best to use POSIX advisory locks, or you are dealing with a dynamically growing mutex tree ... or initializing a file lock mutex to a thread, and each thread checks for a different lock (yuck!) (Because files can be renamed).

I think you are looking at the barrel with some serious corrections. Unfortunately (from what you specified) there is no choice but to make them. In this case, it’s actually easier to debug a stream program written in this way than to debug something with forks, consider yourself happy :)

Answer 4

You can also add some wait condition (pthread_cond_wait) instead of a certain number of nanolayers, which will be signaled if necessary, for example. when a new file opens.