Use multimap with Persistent module handle to avoid IdentityHash coll…

…ision (denoland#1466)

libdeno/binding.cc

@@ -378,21 +378,26 @@ void DenoIsolate::ClearModules() {
  it->second.Reset();
  }
  module_map_.clear();
- module_filename_map_.clear();
+ for (auto it = module_info_map_.begin(); it != module_info_map_.end(); it++) {
+ it->second.second.Reset();
+ }
+ module_info_map_.clear();
 }
 
 void DenoIsolate::RegisterModule(const char* filename,
  v8::Local<v8::Module> module) {
  int id = module->GetIdentityHash();
 
- // v8.h says that identity hash is not necessarily unique. It seems it's quite
- // unique enough for the purposes of O(1000) modules, so we use it as a
- // hashmap key here. The following check is to detect collisions.
- CHECK_EQ(0, module_filename_map_.count(id));
-
- module_filename_map_[id] = filename;
  module_map_.emplace(std::piecewise_construct, std::make_tuple(filename),
  std::make_tuple(isolate_, module));
+
+ // Identity hash is not necessarily unique
+ // Therefore, we store a persistent handle along with filenames
+ // such that we can compare the identites and select the correct module
+ module_info_map_.emplace(
+ std::piecewise_construct, std::make_tuple(id),
+ std::make_tuple(std::piecewise_construct, std::make_tuple(filename),
+ std::make_tuple(isolate_, module)));
 }
 
 v8::MaybeLocal<v8::Module> CompileModule(v8::Local<v8::Context> context,
@@ -468,7 +473,19 @@ v8::MaybeLocal<v8::Module> ResolveCallback(v8::Local<v8::Context> context,
  }
 
  int ref_id = referrer->GetIdentityHash();
- std::string referrer_filename = d->module_filename_map_[ref_id];
+ auto range = d->module_info_map_.equal_range(ref_id);
+ std::string referrer_filename;
+ for (auto it = range.first; it != range.second; ++it) {
+ // it->second: <string, v8::Persistent<v8::Module>>
+ // operator== compares value identities stored in the handles
+ // https://denolib.github.io/v8-docs/include_2v8_8h_source.html#l00487
+ // Due to possibilities of identity hash collision, this is necessary
+ if (it->second.second == referrer) {
+ referrer_filename = it->second.first;
+ break;
+ }
+ }
+ CHECK(referrer_filename.size() != 0);
 
  v8::String::Utf8Value specifier_(isolate, specifier);
  const char* specifier_c = ToCString(specifier_);

libdeno/internal.h

@@ -58,8 +58,9 @@ class DenoIsolate {
  int32_t next_req_id_;
  void* user_data_;
 
- // identity hash -> filename
- std::map<int, std::string> module_filename_map_;
+ // identity hash -> filename, module (avoid hash collision)
+ std::multimap<int, std::pair<std::string, v8::Persistent<v8::Module>>>
+ module_info_map_;
  // filename -> Module
  std::map<std::string, v8::Persistent<v8::Module>> module_map_;
  // Set by deno_resolve_ok