1

I have been working on an open source project for awhile, http://gtkworkbook.sourceforge.net/, and recently ran into an issue that just seems like I am going in circles. I'm pretty sure there is a heap problem but I have been looking at this code too long to figure out exactly what it is.

So, in brief, what I am doing is reallocating a block of memory from N pointers to M pointers while working with a libcsv parser. If there are additional columns I want to increase the maximum size of the array to 2 times the current size. Here's the code currently:


 struct csv_column {
    Sheet * sheet;
    Cell ** array;
    int & array_max;
    int & array_size;
    int row;
    int field;
    char * value;
  };

  static void 
  cb1 (void * s, size_t length, void * data) {
    struct csv_column * column = (struct csv_column *) data;
    int & array_max = column->array_max;

    // Resize the cell array here.
    if (column->field >= array_max) {
      int max = (2 * array_max);
      (column->array) = (Cell **) g_realloc ((column->array), max * sizeof (Cell*));

      for (int ii = array_max; ii array)[column->field] == NULL)
      (column->array)[column->field] = cell_new();

    Cell * cell = (column->array)[column->field];
    cell->set_row (cell, column->row);
    cell->set_column (cell, column->field++);
    cell->set_value_length (cell, s, length);
  }

  CsvParser::CsvParser (Workbook * wb,
            FILE * log,
            int verbosity,
            int maxOfFields) {
    this->wb = wb;
    this->log = log;
    this->verbosity = verbosity;
    this->sizeOfFields = 0;
    this->maxOfFields = maxOfFields;
    this->fields = (Cell **) g_malloc (maxOfFields * sizeof (Cell*));

    for (int ii = 0; ii maxOfFields; ii++)
      this->fields[ii] = NULL;
  }

  CsvParser::~CsvParser (void) {
    for (int ii = 0; ii maxOfFields; ii++) {
      if (this->fields[ii])
        this->fields[ii]->destroy (this->fields[ii]);
    }

    g_free (this->fields);
  }

Here is the valgrind output:

==28476== Thread 9:
==28476== Invalid read of size 8
==28476==    at 0x771AF4F: sheet_method_apply_cellarray (sheet.c:351)
==28476==    by 0xD930DB7: largefile::CsvParser::run(void*) (CsvParser.cpp:147)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)
==28476==  Address 0xbc5d4a8 is 0 bytes inside the accessing pointer's
==28476==  once-legitimate range, a block of size 160 free'd
==28476==    at 0x4C25D4F: free (vg_replace_malloc.c:323)
==28476==    by 0xD9314CA: largefile::cb1(void*, unsigned long, void*) (CsvParser.cpp:57)
==28476==    by 0xDB42681: csv_parse (in /home/jbellone/work/gtkworkbook/lib/libcsv.so)
==28476==    by 0xD930D00: largefile::CsvParser::run(void*) (CsvParser.cpp:136)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)
==28476== 
==28476== Invalid read of size 8
==28476==    at 0x771AF66: sheet_method_apply_cellarray (sheet.c:351)
==28476==    by 0xD930DB7: largefile::CsvParser::run(void*) (CsvParser.cpp:147)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)
==28476==  Address 0xbc5d4a8 is 0 bytes inside the accessing pointer's
==28476==  once-legitimate range, a block of size 160 free'd
==28476==    at 0x4C25D4F: free (vg_replace_malloc.c:323)
==28476==    by 0xD9314CA: largefile::cb1(void*, unsigned long, void*) (CsvParser.cpp:57)
==28476==    by 0xDB42681: csv_parse (in /home/jbellone/work/gtkworkbook/lib/libcsv.so)
==28476==    by 0xD930D00: largefile::CsvParser::run(void*) (CsvParser.cpp:136)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)

sheet.c line 351


   gtk_sheet_set_cell_text (GTK_SHEET (sheet->gtk_sheet),
                 array[ii]->row,
                 array[ii]->column,
                 array[ii]->value->str);

The whole function from sheet.c:


static void
sheet_method_apply_cellarray (Sheet * sheet, 
                  Cell ** array,
                  gint size)
{
  ASSERT (sheet != NULL);
  g_return_if_fail (array != NULL);

  gdk_threads_enter ();

  /* We'll see how this performs for now. In the future we may want to go
     directly into the GtkSheet structures to get a little more performance
     boost (mainly because we should not have to check all the bounds each
     time we want to update). */
  for (gint ii = 0; ii gtk_sheet),
                 array[ii]->row,
                 array[ii]->column,
                 array[ii]->value->str);

    if (!IS_NULLSTR (array[ii]->attributes.bgcolor->str))
      sheet->range_set_background (sheet, 
                   &array[ii]->range, 
                   array[ii]->attributes.bgcolor->str);

    if (!IS_NULLSTR (array[ii]->attributes.fgcolor->str))
      sheet->range_set_foreground (sheet, 
                   &array[ii]->range, 
                   array[ii]->attributes.fgcolor->str);

    /* Clear all of the strings */
    g_string_assign (array[ii]->value, "");
    g_string_assign (array[ii]->attributes.bgcolor, "");
    g_string_assign (array[ii]->attributes.fgcolor, "");
  }

  gdk_threads_leave ();
}

CSV parser thread


  void *
  CsvParser::run (void * null) {
    this->running = true;
    std::queue queue;
    struct csv_parser csv;
    struct csv_column column = {this->wb->sheet_first,
                this->fields,
                this->maxOfFields,
                this->sizeOfFields,
                0,
                0,
                new char [1024]};

    if (csv_init (&csv, CSV_STRICT) != 0) {
      std::cerr running == true) {
      if (this->inputQueue.size() > 0) {

    // Lock, copy, clear, unlock. - Free this up.
    this->inputQueue.lock();
    this->inputQueue.copy (queue);
    this->inputQueue.clear();
    this->inputQueue.unlock();

    while (queue.size() > 0) {
      std::string buf = queue.front(); queue.pop();
      size_t bytes = buf.length();

      if (this->running == false)
        break;

      if ((bytes = csv_parse (&csv, buf.c_str(), bytes, cb1, cb2, &column)) == bytes) {
        if (csv_error (&csv) == CSV_EPARSE) {
          std::cerr wb->sheet_first->apply_array (this->wb->sheet_first,
                                          this->fields,
                                          this->sizeOfFields);

      if (column.row >= (column.sheet)->max_rows)
        column.row = 0;
    }
      } 
      concurrent::Thread::sleep(1);
    }

    return NULL;
  }
jonner
  • 6,331
  • 6
  • 30
  • 28
John Bellone
  • 1,351
  • 1
  • 16
  • 29

1 Answers1

2

My guess is that you are running into a race condition with threading. Cb1 is reallocating the array while your parser is trying to use it. After the realloc the old address is no longer valid and that is where your invalid read is coming from. You'll need to put a lock (perhaps a reader/writer lock) around the array to keep from hitting this problem. If it's possible, try running this code single-threaded to see if the problem reproduces. If it doesn't then it's a threading problem, otherwise it's something else entirely.

Talljoe
  • 14,593
  • 4
  • 43
  • 39
  • That was one of my latest thoughts. I'll test this avenue. – John Bellone Jun 04 '09 at 21:22
  • Actually, I am not sure this is the case now that I think a little more. The CSV parser and cb1 are being executed from the same thread; csv_parse method from libcsv calls cb1 (and cb2) after each column and row are parsed respectively. The call to libworkbook (sheet_method_apply_cellarray) waits on the GDK drawing lock to perform anything. – John Bellone Jun 04 '09 at 21:28
  • I am adding the execution thread where the parser method is invoked; the cb1 method is static to this file. – John Bellone Jun 04 '09 at 21:33
  • 1
    You can also use Valgrind to detect threading-related issues: valgrind --tool=helgrind. – Laurynas Biveinis Jun 05 '09 at 06:35
  • This seems to be the problem, although right now I do not have access to helgrind on the machine I am debugging on. Hopefully I can check later in the week. – John Bellone Jun 10 '09 at 01:26