Serializer enable/disable flags need thread safety.

src/serialize.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 631 matching lines @@
 }
 
 
 ExternalReferenceDecoder::~ExternalReferenceDecoder() {
   for (int type = kFirstTypeCode; type < kTypeCodeCount; ++type) {
     DeleteArray(encodings_[type]);
   }
   DeleteArray(encodings_);
 }
 
-
-bool Serializer::serialization_enabled_ = false;
-bool Serializer::too_late_to_enable_now_ = false;
-
+Atomic32 Serializer::serialization_state_ = SERIALIZER_STATE_UNINITIALIZED;

[Hannes Payer (out of office), 2014/04/17 09:48:10]

AtomicWord

[mvstanton, 2014/04/17 14:18:29]

Done.

 
 class CodeAddressMap: public CodeEventLogger {
  public:
   explicit CodeAddressMap(Isolate* isolate)
       : isolate_(isolate) {
     isolate->logger()->addCodeEventListener(this);
   }
 
   virtual ~CodeAddressMap() {
     isolate_->logger()->removeCodeEventListener(this);
@@ skipping 92 matching lines @@
   }
 
   NameMap address_to_name_map_;
   Isolate* isolate_;
 };
 
 
 CodeAddressMap* Serializer::code_address_map_ = NULL;
 
 
-void Serializer::Enable(Isolate* isolate) {
-  if (!serialization_enabled_) {
-    ASSERT(!too_late_to_enable_now_);
-  }
-  if (serialization_enabled_) return;
-  serialization_enabled_ = true;
+void Serializer::RequestEnable(Isolate* isolate) {
   isolate->InitializeLoggingAndCounters();
   code_address_map_ = new CodeAddressMap(isolate);
 }
 
 
-void Serializer::Disable() {
-  if (!serialization_enabled_) return;
-  serialization_enabled_ = false;
-  delete code_address_map_;
-  code_address_map_ = NULL;
+void Serializer::InitializeOncePerProcess() {
+  ASSERT(NoBarrier_Load(&serialization_state_) ==
+         SERIALIZER_STATE_UNINITIALIZED);
+  SerializationState state = code_address_map_
+      ? SERIALIZER_STATE_ENABLED
+      : SERIALIZER_STATE_DISABLED;
+  NoBarrier_Store(&serialization_state_, state);

[Hannes Payer (out of office), 2014/04/17 09:48:10]

Can we have a comment here why this works and why no barrier loads/stores are
fine.
Can there by a enable/disable race?

[mvstanton, 2014/04/17 14:18:29]

Yes, done, and no.

 }
 
 
+void Serializer::TearDown() {
+  ASSERT(NoBarrier_Load(&serialization_state_) !=
+         SERIALIZER_STATE_UNINITIALIZED);
+  if (code_address_map_) {
+    ASSERT(NoBarrier_Load(&serialization_state_) ==
+           SERIALIZER_STATE_ENABLED);
+    delete code_address_map_;
+    code_address_map_ = NULL;
+  }
+
+  NoBarrier_Store(&serialization_state_, SERIALIZER_STATE_UNINITIALIZED);

[Hannes Payer (out of office), 2014/04/17 09:48:10]

The assumption is that this method is just invoked by one thread, right?
Otherwise it wouldn't work. Please add a comment to explain the global picture.

[mvstanton, 2014/04/17 14:18:29]

Good point, done.

+}
+
+
 Deserializer::Deserializer(SnapshotByteSource* source)
     : isolate_(NULL),
       source_(source),
       external_reference_decoder_(NULL) {
   for (int i = 0; i < LAST_SPACE + 1; i++) {
     reservations_[i] = kUninitializedReservation;
   }
 }
 
 
@@ skipping 1086 matching lines @@
 
 bool SnapshotByteSource::AtEOF() {
   if (0u + length_ - position_ > 2 * sizeof(uint32_t)) return false;
   for (int x = position_; x < length_; x++) {
     if (data_[x] != SerializerDeserializer::nop()) return false;
   }
   return true;
 }
 
 } }  // namespace v8::internal