[Urgent] Fix Node.js and use newer version

[piranna]

We are currently using Node.js v0.11.14 because newer versions can't be used as PID 1. The problem is identified to be on v8 when it was changed from 3.26.33 to 3.28.73, but I have been not able to find what exactly change introduced it because Node.js use a patched v8, so git bisect can't be used alone.

The problem consists in the fact that instead showing Node.js a REPL or exec a script when used as PID 1, it seems to enter in a loop that hangs the CPU to the 100% and don't exec anymore nor crash. This is extremely important, since now with Node.js 5 and nan 2 supporting it there are some modules that their latest versions can't be used on NodeOS, and the number will increase. If someone have the time/knowledges here I will hugely thanks him, with knowing what v8 version introducced the bug so we can isolate the problem is more than enough.

Thank you to everybody in advance.

[piranna]

I've managed to create a diff with the changes introducced by Node.js team on top of v8 3.28.73. Sometimes I've found errors related to v8.h not found and in fact its path is changed, so I'll continue over there to see if I can find where the problem is in the upstream v8. I'll try to apply this patch in previous versions to reduce the range of commits.

--- a/include/v8.h
+++ b/include/v8.h
@@ -4187,6 +4187,17 @@
   static Isolate* GetCurrent();

   /**
+   * Custom callback used by embedders to help V8 determine if it should abort
+   * when it throws and no internal handler can catch the exception.
+   * If FLAG_abort_on_uncaught_exception is true, then V8 will abort if either:
+   * - no custom callback is set.
+   * - the custom callback set returns true.
+   * Otherwise it won't abort.
+   */
+  typedef bool (*abort_on_uncaught_exception_t)();
+  void SetAbortOnUncaughtException(abort_on_uncaught_exception_t callback);
+
+  /**
    * Methods below this point require holding a lock (using Locker) in
    * a multi-threaded environment.
    */
--- a/src/base/platform/platform-posix.cc
+++ b/src/base/platform/platform-posix.cc
@@ -321,11 +321,15 @@


 int OS::GetCurrentThreadId() {
-#if defined(ANDROID)
+#if V8_OS_MACOSX
+  return static_cast<int>(pthread_mach_thread_np(pthread_self()));
+#elif V8_OS_LINUX
   return static_cast<int>(syscall(__NR_gettid));
-#else
-  return static_cast<int>(syscall(SYS_gettid));
-#endif  // defined(ANDROID)
+#elif V8_OS_ANDROID
+  return static_cast<int>(gettid());
+#else
+  return static_cast<int>(pthread_self());
+#endif
 }


@@ -604,9 +608,8 @@


 void Thread::YieldCPU() {
-  int result = sched_yield();
-  DCHECK_EQ(0, result);
-  USE(result);
+  const timespec delay = { 0, 1 };
+  nanosleep(&delay, NULL);
 }


--- a/src/base/platform/platform.h
+++ b/src/base/platform/platform.h
@@ -35,6 +35,7 @@
 int signbit(double x);
 }
 # endif
+#include <alloca.h>
 #endif

 #if V8_OS_QNX
--- a/src/api.cc
+++ b/src/api.cc
@@ -6731,7 +6731,6 @@
   }
 }

-
 int v8::Isolate::ContextDisposedNotification() {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
   return isolate->heap()->NotifyContextDisposed();
--- a/src/codegen.cc
+++ b/src/codegen.cc
@@ -190,7 +190,7 @@
             function->end_position() - function->start_position() + 1;
         for (int i = 0; i < source_len; i++) {
           if (stream.HasMore()) {
-            os << AsUC16(stream.GetNext());
+            os << AsReversiblyEscapedUC16(stream.GetNext());
           }
         }
         os << "\n\n";
--- a/src/debug-debugger.js
+++ b/src/debug-debugger.js
@@ -430,7 +430,7 @@
   if (IS_NULL(position)) return;

   // Create a break point object and set the break point.
-  break_point = MakeBreakPoint(position, this);
+  var break_point = MakeBreakPoint(position, this);
   break_point.setIgnoreCount(this.ignoreCount());
   var actual_position = %SetScriptBreakPoint(script, position,
                                              this.position_alignment_,
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -3498,7 +3498,7 @@
               shared->end_position() - shared->start_position() + 1;
           for (int i = 0; i < source_len; i++) {
             if (stream.HasMore()) {
-              os << AsUC16(stream.GetNext());
+              os << AsReversiblyEscapedUC16(stream.GetNext());
             }
           }
         }
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1090,10 +1090,9 @@
         thread_local_top()->pending_message_end_pos_ = location->end_pos();
       }

-      // If the abort-on-uncaught-exception flag is specified, abort on any
-      // exception not caught by JavaScript, even when an external handler is
-      // present.  This flag is intended for use by JavaScript developers, so
-      // print a user-friendly stack trace (not an internal one).
+      // If the abort-on-uncaught-exception flag is specified, and if the
+      // exception is not caught by JavaScript (even when an external handler is
+      // present).
       if (fatal_exception_depth == 0 &&
           FLAG_abort_on_uncaught_exception &&
           (report_exception || can_be_caught_externally)) {
@@ -1297,7 +1296,6 @@
   stack_trace_for_uncaught_exceptions_frame_limit_ = frame_limit;
   stack_trace_for_uncaught_exceptions_options_ = options;
 }
-

 Handle<Context> Isolate::native_context() {
   return handle(context()->native_context());
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -1326,6 +1326,7 @@

   v8::Isolate::UseCounterCallback use_counter_callback_;

+
   friend class ExecutionAccess;
   friend class HandleScopeImplementer;
   friend class IsolateInitializer;
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -11417,7 +11417,10 @@

   os << "Instructions (size = " << instruction_size() << ")\n";
   // TODO(svenpanne) The Disassembler should use streams, too!
-  Disassembler::Decode(stdout, this);
+  {
+    CodeTracer::Scope trace_scope(GetIsolate()->GetCodeTracer());
+    Disassembler::Decode(trace_scope.file(), this);
+  }
   os << "\n";

   if (kind() == FUNCTION) {
--- a/src/ostreams.cc
+++ b/src/ostreams.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.

 #include <algorithm>
+#include <cctype>
 #include <cmath>

 #include "src/base/platform/platform.h"  // For isinf/isnan with MSVC
@@ -163,11 +164,21 @@
 }


+OStream& operator<<(OStream& os, const AsReversiblyEscapedUC16& c) {
+  char buf[10];
+  const char* format =
+      (std::isprint(c.value) || std::isspace(c.value)) && c.value != '\\'
+          ? "%c"
+          : (c.value <= 0xff) ? "\\x%02x" : "\\u%04x";
+  snprintf(buf, sizeof(buf), format, c.value);
+  return os << buf;
+}
+
+
 OStream& operator<<(OStream& os, const AsUC16& c) {
   char buf[10];
-  const char* format = (0x20 <= c.value && c.value <= 0x7F)
-                           ? "%c"
-                           : (c.value <= 0xff) ? "\\x%02x" : "\\u%04x";
+  const char* format =
+      std::isprint(c.value) ? "%c" : (c.value <= 0xff) ? "\\x%02x" : "\\u%04x";
   snprintf(buf, sizeof(buf), format, c.value);
   return os << buf;
 }
--- a/src/ostreams.h
+++ b/src/ostreams.h
@@ -117,13 +117,26 @@
 };


-// A wrapper to disambiguate uint16_t and uc16.
+// Wrappers to disambiguate uint16_t and uc16.
 struct AsUC16 {
   explicit AsUC16(uint16_t v) : value(v) {}
   uint16_t value;
 };


+struct AsReversiblyEscapedUC16 {
+  explicit AsReversiblyEscapedUC16(uint16_t v) : value(v) {}
+  uint16_t value;
+};
+
+
+// Writes the given character to the output escaping everything outside of
+// printable/space ASCII range. Additionally escapes '\' making escaping
+// reversible.
+OStream& operator<<(OStream& os, const AsReversiblyEscapedUC16& c);
+
+// Writes the given character to the output escaping everything outside
+// of printable ASCII range.
 OStream& operator<<(OStream& os, const AsUC16& c);
 } }  // namespace v8::internal

--- a/tools/gyp/v8.gyp
+++ b/tools/gyp/v8.gyp
@@ -1250,7 +1250,7 @@
         ['OS=="solaris"', {
             'link_settings': {
               'libraries': [
-                '-lnsl',
+                '-lnsl -lrt',
             ]},
             'sources': [
               '../../src/base/platform/platform-solaris.cc',
--- a/tools/gen-postmortem-metadata.py
+++ b/tools/gen-postmortem-metadata.py
@@ -70,8 +70,6 @@
     { 'name': 'ExternalStringTag',      'value': 'kExternalStringTag' },
     { 'name': 'SlicedStringTag',        'value': 'kSlicedStringTag' },

-    { 'name': 'FailureTag',             'value': 'kFailureTag' },
-    { 'name': 'FailureTagMask',         'value': 'kFailureTagMask' },
     { 'name': 'HeapObjectTag',          'value': 'kHeapObjectTag' },
     { 'name': 'HeapObjectTagMask',      'value': 'kHeapObjectTagMask' },
     { 'name': 'SmiTag',                 'value': 'kSmiTag' },
@@ -94,8 +92,6 @@
         'value': 'DescriptorArray::kFirstIndex' },
     { 'name': 'prop_type_field',
         'value': 'FIELD' },
-    { 'name': 'prop_type_first_phantom',
-        'value': 'TRANSITION' },
     { 'name': 'prop_type_mask',
         'value': 'PropertyDetails::TypeField::kMask' },
     { 'name': 'prop_index_mask',
@@ -120,9 +116,9 @@
         'value': 'DICTIONARY_ELEMENTS' },

     { 'name': 'bit_field2_elements_kind_mask',
-       'value': 'Map::kElementsKindMask' },
+        'value': 'Map::ElementsKindBits::kMask' },
     { 'name': 'bit_field2_elements_kind_shift',
-       'value': 'Map::kElementsKindShift' },
+        'value': 'Map::ElementsKindBits::kShift' },
     { 'name': 'bit_field3_dictionary_map_shift',
         'value': 'Map::DictionaryMap::kShift' },

@@ -196,9 +192,9 @@
  * This file is generated by %s.  Do not edit directly.
  */

-#include "v8.h"
-#include "frames.h"
-#include "frames-inl.h" /* for architecture-specific frame constants */
+#include "src/v8.h"
+#include "src/frames.h"
+#include "src/frames-inl.h" /* for architecture-specific frame constants */

 using namespace v8::internal;

[piranna]

By the way: I can confirm that Node.js v0.11.15 works when exec alone with QEmu user mode, but NOT when used as PID 1 on NodeOS with QEmu.

[piranna]

I have been able to isolate the problem between v8 versions 3.27.0 and 3.27.14, I'll to do the last mile in the next days. If someone wants to check it, the diff applied to 3.27.14 is this:

diff --git a/include/v8.h b/include/v8.h
index 656aa35..06a69c3 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -4154,6 +4154,17 @@ class V8_EXPORT Isolate {
   static Isolate* GetCurrent();

   /**
+   * Custom callback used by embedders to help V8 determine if it should abort
+   * when it throws and no internal handler can catch the exception.
+   * If FLAG_abort_on_uncaught_exception is true, then V8 will abort if either:
+   * - no custom callback is set.
+   * - the custom callback set returns true.
+   * Otherwise it won't abort.
+   */
+  typedef bool (*abort_on_uncaught_exception_t)();
+  void SetAbortOnUncaughtException(abort_on_uncaught_exception_t callback);
+
+  /**
    * Methods below this point require holding a lock (using Locker) in
    * a multi-threaded environment.
    */
diff --git a/src/debug-debugger.js b/src/debug-debugger.js
index 5462b53..b43ab01 100644
--- a/src/debug-debugger.js
+++ b/src/debug-debugger.js
@@ -428,7 +428,7 @@ ScriptBreakPoint.prototype.set = function (script) {
   if (IS_NULL(position)) return;

   // Create a break point object and set the break point.
-  break_point = MakeBreakPoint(position, this);
+  var break_point = MakeBreakPoint(position, this);
   break_point.setIgnoreCount(this.ignoreCount());
   var actual_position = %SetScriptBreakPoint(script, position,
                                              this.position_alignment_,
diff --git a/src/isolate.cc b/src/isolate.cc
index d204726..9113bb1 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1097,10 +1097,9 @@ void Isolate::DoThrow(Object* exception, MessageLocation* location) {
         thread_local_top()->pending_message_end_pos_ = location->end_pos();
       }

-      // If the abort-on-uncaught-exception flag is specified, abort on any
-      // exception not caught by JavaScript, even when an external handler is
-      // present.  This flag is intended for use by JavaScript developers, so
-      // print a user-friendly stack trace (not an internal one).
+      // If the abort-on-uncaught-exception flag is specified, and if the
+      // exception is not caught by JavaScript (even when an external handler is
+      // present).
       if (fatal_exception_depth == 0 &&
           FLAG_abort_on_uncaught_exception &&
           (report_exception || can_be_caught_externally)) {
@@ -1311,7 +1310,6 @@ void Isolate::SetCaptureStackTraceForUncaughtExceptions(
   stack_trace_for_uncaught_exceptions_options_ = options;
 }

-
 Handle<Context> Isolate::native_context() {
   return Handle<Context>(context()->global_object()->native_context());
 }
diff --git a/src/platform.h b/src/platform.h
index 7d857cc..84d842c 100644
--- a/src/platform.h
+++ b/src/platform.h
@@ -34,6 +34,7 @@ namespace std {
 int signbit(double x);
 }
 # endif
+#include <alloca.h>
 #endif

 #if V8_OS_QNX
diff --git a/tools/gen-postmortem-metadata.py b/tools/gen-postmortem-metadata.py
index fff2e34..f00800e 100644
--- a/tools/gen-postmortem-metadata.py
+++ b/tools/gen-postmortem-metadata.py
@@ -70,8 +70,6 @@ consts_misc = [
     { 'name': 'ExternalStringTag',      'value': 'kExternalStringTag' },
     { 'name': 'SlicedStringTag',        'value': 'kSlicedStringTag' },

-    { 'name': 'FailureTag',             'value': 'kFailureTag' },
-    { 'name': 'FailureTagMask',         'value': 'kFailureTagMask' },
     { 'name': 'HeapObjectTag',          'value': 'kHeapObjectTag' },
     { 'name': 'HeapObjectTagMask',      'value': 'kHeapObjectTagMask' },
     { 'name': 'SmiTag',                 'value': 'kSmiTag' },
@@ -84,8 +82,6 @@ consts_misc = [
         'value': 'DescriptorArray::kFirstIndex' },
     { 'name': 'prop_type_field',
         'value': 'FIELD' },
-    { 'name': 'prop_type_first_phantom',
-        'value': 'TRANSITION' },
     { 'name': 'prop_type_mask',
         'value': 'PropertyDetails::TypeField::kMask' },

diff --git a/tools/gyp/v8.gyp b/tools/gyp/v8.gyp
index b563f33..daaca6f 100644
--- a/tools/gyp/v8.gyp
+++ b/tools/gyp/v8.gyp
@@ -954,7 +954,7 @@
         ['OS=="solaris"', {
             'link_settings': {
               'libraries': [
-                '-lnsl',
+                '-lnsl -lrt',
             ]},
             'sources': [
               '../../src/platform-solaris.cc',
diff --git a/tools/v8.xcodeproj/README.txt b/tools/v8.xcodeproj/README.txt
deleted file mode 100644
index e064ff6..0000000
--- a/tools/v8.xcodeproj/README.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-The Xcode project for V8 has been retired. If an Xcode project
-is needed for building on a Mac there is the option of using GYP to
-generate it. Please look in the build directory in the root of the
-V8 project. It contains the required infrastructure and a README.txt
-file explaining how to get started.
-
-Generating Xcode projects using GYP is how the Chromium
-project integrated V8 into the Mac build.
-
-The main build system for V8 is still SCons, see
-http://code.google.com/apis/v8/build.html for details.
-- 
2.1.4

[piranna]

I have get out of ideas. By applying the changes from v8 3.28.73 backwards up to 3.26.33 it doesn't work, since also this last one gets compiled but don't show a REPL when used as PID 1, while using that version with the Node.js patches does. Doing the other way I get only to make it work up to v8 3.27.0, getting segmentation faults after that. To worse things, the commit that introducced v8 3.28.73 that worked before now don't compile, don't know why the difference of the behaviour.

I'm honestly out of ideas, anybody can help me? The only one I could think about is to get the backwards patched version and try to make them work again by fixing the patches... :-/

[zodern]

I have never done anything like this before so I have no ideas. If there is anything I can do to help, please let me know.

Do you still think it is between v8 3.27.0 and 3.27.14?

[piranna]

Do you still think it is between v8 3.27.0 and 3.27.14?

Now I'm not sure, because as I told before, the patches backward up to v8 3.26.33 are failling. I'm only sure it's not on 3.27.0 when applied in order, subsequent ones throw a segmentation fault when compiling. Maybe the problem is on the patches thenselves... but since the applied ones on 3.28.73 are failking to compile and before not, maybe I'm missing a step done before... I'll try to get back the patch applied to v8 3.26.33 and make it work, but the differences were really subtle to make such a difference :-/

[santigimeno]

@piranna is the problem reproducible in docker? I've tried running node 4.2.2 with PID 1 in a docker instance and had no problems

[piranna]

@piranna is the problem reproducible in docker? I've tried running node 4.2.2 with PID 1 in a docker instance and had no problems

I haven't tried in Docker, only on QEmu and real hardware. Docker don't emulate a real computer like QEmu does but instead provides an isolate container where it makes normal processes to think they are PID 1, maybe that's the difference that makes it work there...

[Xe]

Please instead consider using sinit as your core system init process. Software behaves slightly differently when running as PID1 than other processes on the system, even more so when you are the actual /sbin/init that is being directly called by the kernel at boot. It is much better to use something like sinit where things are easier to prove.

[piranna]

Honestly, a really interesting suggestion :-) I disagree on using it due to the fact it's an "obscure" C compiled binary and the system would not be "pure" (take in account that also /usr/bin/env is a Node.js module too!)... but I must to admit that if it works it could be a valid solution, since it would allow us to by-pass that problem and use newer Node.js versions until a solution arises and it could be used alone. You know, if you get a closed door, open a window and jump... :-P Maybe we where thinking so much outside the box we were not aware of an obvious "inside the box" solution :-D

[piranna]

@Xe, I've open an issue to investigate the viability of your suggestion. Thank you! :-)

[piranna]

The usage of a standalone C init has not fixed it, so it's not a matter of Node.js being executed as PID 1 and needs to be searched the problem in other place :-( Maybe Node.js now needs some info from /dev or /proc and that's the reason it enter an infinite loop... Don't know. Only thing I can think is that since this also happen when trying to use latest Node.js version as /init on Ubuntu and since we are now using an independent C init command, maybe we could check if it works adding Node.js to a minimal bootable Ubuntu initram and start removing fragments to see if it works and how far we could go.

Ideas? Comments?