gnuastro-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[gnuastro-commits] master 70b91ab: NoiseChisel issues with memory mappin


From: Mohammad Akhlaghi
Subject: [gnuastro-commits] master 70b91ab: NoiseChisel issues with memory mapping fixed
Date: Sat, 16 Sep 2017 07:05:48 -0400 (EDT)

branch: master
commit 70b91abc2cdb34a5aa1a7a9e890490a200d456ad
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    NoiseChisel issues with memory mapping fixed
    
    When running NoiseChisel with `--minmapsize=0' (to not use the RAM at all
    for arrays), a segmentation fault occured. It was because of changing the
    array pointer in one place without correctly changing the memory mapped
    filename. This issue is now corrected with this commit.
    
    ALso, I noticed that in `gal_data_free_contents', we weren't setting block
    to NULL and `mmapname' to NULL (after freeing it).
    
    Finally, an easier way to ask `./tmpfs-config-make' to compile in debugging
    mode was defined. Until now, it was necessary to set/remove a comment close
    to the end of the line. But now, through a variable (`debug_noshared') at
    the start of the file, this behavior can easily be set.
---
 NEWS                           |  2 ++
 bin/noisechisel/clumps.c       |  1 +
 bin/noisechisel/detection.c    | 11 ++++++++++-
 bin/noisechisel/segmentation.c |  9 +++++++--
 bin/noisechisel/threshold.c    |  1 -
 doc/announce-acknowledge.txt   |  1 +
 lib/data.c                     |  4 ++++
 lib/tile.c                     |  2 +-
 tmpfs-config-make              |  7 ++++++-
 9 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 9f9922f..a5b442b 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,8 @@ GNU Astronomy Utilities NEWS                          -*- 
outline -*-
   Arithmetic not accounting for integer blank pixels in binary operators
   (bug #52014).
 
+  NoiseChisel segfault when memory mapping to a file (bug #52043).
+
 
 
 
diff --git a/bin/noisechisel/clumps.c b/bin/noisechisel/clumps.c
index df7b383..bd1cf09 100644
--- a/bin/noisechisel/clumps.c
+++ b/bin/noisechisel/clumps.c
@@ -1304,6 +1304,7 @@ clumps_true_find_sn_thresh(struct noisechiselparams *p)
                            p->ltl.tottiles, p->cp.numthreads);
     }
 
+
   /* Destroy the mutex if it was initialized. */
   if( p->cp.numthreads>1 && (p->checksegmentation || p->checkclumpsn) )
     pthread_mutex_destroy(&clprm.labmutex);
diff --git a/bin/noisechisel/detection.c b/bin/noisechisel/detection.c
index 2c5b88b..fbe15cb 100644
--- a/bin/noisechisel/detection.c
+++ b/bin/noisechisel/detection.c
@@ -356,7 +356,16 @@ detection_pseudo_find(struct noisechiselparams *p, 
gal_data_t *workbin,
 
       /* Clean up: the array in `bin' should just be replaced with that in
          `workbin' because it is used in later steps. */
-      free(workbin->array);
+      if(workbin->mmapname)
+        {
+          /* Delete the memory mapped file and set the filename of `bin'
+             for `workbin'. */
+          remove(workbin->mmapname);
+          workbin->mmapname=bin->mmapname;
+          bin->mmapname=NULL;
+        }
+      else
+        free(workbin->array);
       workbin->array=bin->array;
       bin->name=bin->array=NULL;
       gal_data_free(bin);
diff --git a/bin/noisechisel/segmentation.c b/bin/noisechisel/segmentation.c
index 0b6bd65..4cbfb0e 100644
--- a/bin/noisechisel/segmentation.c
+++ b/bin/noisechisel/segmentation.c
@@ -411,6 +411,7 @@ segmentation_on_threads(void *in_prm)
       /* Find the clumps over this region. */
       clumps_oversegment(&cltprm);
 
+
       /* Make the clump S/N table. This table is made before (possibly)
          stopping the process (if a check is requested). This is because if
          the user has also asked for a check image, we can break out of the
@@ -424,6 +425,7 @@ segmentation_on_threads(void *in_prm)
         clumps_make_sn_table(&cltprm);
       else cltprm.sn=&clprm->sn[ cltprm.id ];
 
+
       /* If the user wanted to check the segmentation steps or the clump
          S/N values in a table, then we have to stop the process at this
          point. */
@@ -435,6 +437,7 @@ segmentation_on_threads(void *in_prm)
       gal_data_free(topinds);
       if(clprm->step==2) continue;
 
+
       /* Set the internal (with the detection) clump and object
          labels. Segmenting a detection into multiple objects is only
          defined when there is more than one true clump over the
@@ -760,8 +763,9 @@ segmentation_detections(struct noisechiselparams *p)
 
             default:
               error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at %s so "
-                    "we can address the issue. The value %d is not recognized "
-                    "for clprm.step", __func__, PACKAGE_BUGREPORT, clprm.step);
+                    "we can address the issue. The value %d is not "
+                    "recognized for clprm.step", __func__, PACKAGE_BUGREPORT,
+                    clprm.step);
             }
 
           /* Write the demonstration array into the check image. The
@@ -791,6 +795,7 @@ segmentation_detections(struct noisechiselparams *p)
                            p->cp.numthreads);
     }
 
+
   /* Save the final number of objects and clumps. */
   p->numclumps=clprm.totclumps;
   p->numobjects=clprm.totobjects;
diff --git a/bin/noisechisel/threshold.c b/bin/noisechisel/threshold.c
index d7da847..34ae8fd 100644
--- a/bin/noisechisel/threshold.c
+++ b/bin/noisechisel/threshold.c
@@ -266,7 +266,6 @@ threshold_interp_smooth(struct noisechiselparams *p, 
gal_data_t **first,
     error(EXIT_FAILURE, 0, "%s: `third' must not have any `next' pointer.",
           __func__);
 
-
   /* Do the interpolation of both arrays. */
   (*first)->next = *second;
   if(third) (*second)->next = *third;
diff --git a/doc/announce-acknowledge.txt b/doc/announce-acknowledge.txt
index b76e5bb..880c69c 100644
--- a/doc/announce-acknowledge.txt
+++ b/doc/announce-acknowledge.txt
@@ -2,3 +2,4 @@ This file is meant to keep the names of the people who's help 
must be
 acknowledged in the next release.
 
 Takashi Ichikawa
+David Valls-Gabaud
diff --git a/lib/data.c b/lib/data.c
index ba7ff40..d553fac 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -432,6 +432,9 @@ gal_data_free_contents(gal_data_t *data)
 
       /* Free the file name space. */
       free(data->mmapname);
+
+      /* Set the name pointer to NULL since it has been freed. */
+      data->mmapname=NULL;
     }
   else
     if(data->array && data->block==NULL) free(data->array);
@@ -505,6 +508,7 @@ gal_data_array_calloc(size_t size)
       out[i].wcs        = NULL;
       out[i].mmapname   = NULL;
       out[i].next       = NULL;
+      out[i].block      = NULL;
       out[i].name = out[i].unit = out[i].comment = NULL;
       out[i].disp_fmt = out[i].disp_width = out[i].disp_precision = -1;
     }
diff --git a/lib/tile.c b/lib/tile.c
index 05c8eac..c972ea5 100644
--- a/lib/tile.c
+++ b/lib/tile.c
@@ -1244,7 +1244,7 @@ gal_tile_full_free_contents(struct 
gal_tile_two_layer_params *tl)
   if(tl->permutation)   free(tl->permutation);
   if(tl->firsttsize)    free(tl->firsttsize);
 
-  /* Free the arrays of `gal_data_c' for each tile and channel. */
+  /* Free the arrays of `gal_data_t' for each tile and channel. */
   if(tl->tiles)    gal_data_array_free(tl->tiles,    tl->tottiles,    0);
   if(tl->channels) gal_data_array_free(tl->channels, tl->totchannels, 0);
 }
diff --git a/tmpfs-config-make b/tmpfs-config-make
index 019de25..41cc305 100755
--- a/tmpfs-config-make
+++ b/tmpfs-config-make
@@ -30,6 +30,7 @@
 # Set the variables:
 NUM_THREADS=8
 TMPDIR=/dev/shm
+debug_noshared=0
 
 
 
@@ -123,7 +124,11 @@ cd $build_dir
 # (in `configure.ac'), we have set optimization flags which have to be
 # cancelled in debugging.
 if [ ! -f Makefile ]; then
-    $srcdir/configure --srcdir=$srcdir #CFLAGS="-g -O0" --disable-shared
+    if [ x$debug_noshared = x1 ]; then
+        $srcdir/configure --srcdir=$srcdir CFLAGS="-g -O0" --disable-shared
+    else
+        $srcdir/configure --srcdir=$srcdir
+    fi
 fi
 
 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]