justinethier / cyclone

:cyclone: A brand-new compiler that allows practical application development using R7RS Scheme. We provide modern features and a stable system capable of generating fast native binaries.
http://justinethier.github.io/cyclone/
MIT License
823 stars 42 forks source link

Race condition in symbol creation functions #450

Closed Skyb0rg007 closed 3 years ago

Skyb0rg007 commented 3 years ago

The current runtime implementation does not properly handle parallel symbol creation. The problematic code is here.

The issue is that a separate thread can create the symbol in-between lines 472 and 473. This can be seen in the gist here. In the gist, two threads race to create the same symbol. When the race occurs, two symbols of the same value are created, though they do not compare equal (since their memory addresses are not the same).

An example fix is the following code, added to add_symbol:

object add_symbol(symbol_type * psym)
{
  pthread_mutex_lock(&symbol_table_lock);       // Only 1 "writer" allowed
  bool inserted = set_insert(&symbol_table, psym);
  pthread_mutex_unlock(&symbol_table_lock);
  if (!inserted) {
    object sym = find_symbol_by_name(psym->desc);
    free(psym->desc);
    free(psym);
    return sym;
  } else {
    return psym;
  }
}

There may need to be additional changes, but this solution would work to prevent the race from occuring.

justinethier commented 3 years ago

Thanks for the detailed report @Skyb0rg007 Let me spend some time investigating to see if anything else needs to be changed, and then we can patch this up.

justinethier commented 3 years ago

Alright, this is working much better now with your fix, and will be included in the next release. Thanks again for the great work finding and writing this up!