[LibOS] Fix and add a test case for resource leak on file close

LibOS/shim/include/shim_vma.h

@@ -128,12 +128,12 @@ bool is_in_adjacent_vmas(void* addr, size_t length);
  * Note: the first argument is "top_addr" because the search is top-down.
  */
 void* bkeep_unmapped(void* top_addr, void* bottom_addr, size_t length, int prot, int flags,
-                     struct shim_handle* file, off_t offset, const char* comment);
+                     off_t offset, const char* comment);
 
-static inline void* bkeep_unmapped_any(size_t length, int prot, int flags, struct shim_handle* file,
+static inline void* bkeep_unmapped_any(size_t length, int prot, int flags,
                                        off_t offset, const char* comment) {
     return bkeep_unmapped(PAL_CB(user_address.end), PAL_CB(user_address.start), length, prot, flags,
-                          file, offset, comment);
+                          offset, comment);
 }
 
 void* bkeep_unmapped_heap(size_t length, int prot, int flags, struct shim_handle* file,

LibOS/shim/src/bookkeep/shim_vma.c

@@ -850,12 +850,11 @@ static void * __bkeep_unmapped (void * top_addr, void * bottom_addr,
 }
 
 void * bkeep_unmapped (void * top_addr, void * bottom_addr, size_t length,
-                       int prot, int flags, struct shim_handle * file,
-                       off_t offset, const char * comment)
+                       int prot, int flags, off_t offset, const char * comment)
 {
     lock(&vma_list_lock);
     void * addr = __bkeep_unmapped(top_addr, bottom_addr, length, prot, flags,
-                                   file, offset, comment);
+                                   NULL, offset, comment);
     assert_vma_list();
     __restore_reserved_vmas();
     unlock(&vma_list_lock);

LibOS/shim/src/fs/chroot/fs.c

@@ -659,9 +659,8 @@ static inline int __map_buffer (struct shim_handle * hdl, size_t size)
     while (mapoff + maplen < file->marker + size)
         maplen *= 2;
 
-    /* create the bookkeeping before allocating the memory */
-    void * mapbuf = bkeep_unmapped_any(maplen, prot, flags, hdl, mapoff,
-                                       "filebuf");
+    /* Create the bookkeeping before allocating the memory. */
+    void * mapbuf = bkeep_unmapped_any(maplen, prot, flags, mapoff, "filebuf");
     if (!mapbuf)
         return -ENOMEM;
 

LibOS/shim/src/shim_checkpoint.c

@@ -878,7 +878,7 @@ static void * cp_alloc (struct shim_cp_store * store, void * addr, size_t size)
          * top of the virtual address space.
          */
         addr = bkeep_unmapped_any(size + reserve_size, PROT_READ|PROT_WRITE,
-                                  CP_VMA_FLAGS, NULL, 0, "cpstore");
+                                  CP_VMA_FLAGS, 0, "cpstore");
         if (!addr)
             return NULL;
 
@@ -1213,8 +1213,8 @@ int do_migration (struct newproc_cp_header * hdr, void ** cpptr)
 
     if (!base) {
         base = bkeep_unmapped_any(ALIGN_UP(size),
-                                  PROT_READ|PROT_WRITE, CP_VMA_FLAGS,
-                                  NULL, 0, "cpstore");
+                                  PROT_READ|PROT_WRITE, CP_VMA_FLAGS, 0,
+                                  "cpstore");
         if (!base)
             return -ENOMEM;
 

LibOS/shim/src/shim_init.c

@@ -482,7 +482,7 @@ int init_manifest (PAL_HANDLE manifest_handle)
         size = attr.pending_size;
         map_size = ALIGN_UP(size);
         addr = bkeep_unmapped_any(map_size, PROT_READ, MAP_FLAGS,
-                                  NULL, 0, "manifest");
+                                  0, "manifest");
         if (!addr)
             return -ENOMEM;
 

LibOS/shim/src/shim_malloc.c

@@ -61,12 +61,12 @@ void* __system_malloc(size_t size) {
 
     /*
      * If vmas are initialized, we need to request a free address range
-     * using bkeep_unmapped_any().  The current mmap code uses this function
+     * using bkeep_unmapped_any(). The current mmap code uses this function
      * to synchronize all address allocation, via a "publication"
-     * pattern.  It is not safe to just call DkVirtualMemoryAlloc directly
+     * pattern. It is not safe to just call DkVirtualMemoryAlloc directly
      * without reserving the vma region first.
      */
-    addr = bkeep_unmapped_any(alloc_size, PROT_READ | PROT_WRITE, flags, NULL, 0, "slab");
+    addr = bkeep_unmapped_any(alloc_size, PROT_READ | PROT_WRITE, flags, 0, "slab");
 
     if (!addr)
         return NULL;

LibOS/shim/test/regression/fdleak.c

@@ -0,0 +1,25 @@
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+/* This is supposed to expose resource leaks where close()d files are not
+   properly cleaned up. */
+
+int main(int argc, char** argv) {
+    for (int i = 0; i < 10000; i++) {
+        int fd = open(argv[0], O_RDONLY);
+        if (fd == -1) abort();
+        char buf[1024];
+        ssize_t read_ret = read(fd, buf, sizeof(buf));
+        if (read_ret == -1) abort();
+        int ret = close(fd);
+        if (ret == -1) abort();
+    }
+
+    puts("Test succeeded.");
+
+    return 0;
+}

LibOS/shim/test/regression/test_libos.py

@@ -301,6 +301,10 @@ class TC_40_FileSystem(RegressionTestCase):
         # proc/cpuinfo Linux-based formatting
         self.assertIn('cpuinfo test passed', stdout)
 
+    def test_030_fdleak(self):
+        stdout, stderr = self.run_binary(['fdleak'], timeout=10)
+        self.assertIn("Test succeeded.", stdout)
+
 class TC_80_Socket(RegressionTestCase):
     def test_000_getsockopt(self):
         stdout, stderr = self.run_binary(['getsockopt'])