There are hundreds of thousands of google search results (hits) about NodeJS crashing and core dumping and how to keep your server up and running. I see Chrome throw up "Aw snap" pages frequently enough. I've seen segfault messages in apache logfiles.
The fact that SilkJS can crash is fine because software does crash. A bug is a bug. Squash it. We're engineers, we are supposed to make things work. As I see it, the API you exposed in your header files and documentation is something of a contract, and I abided by it. My code has worked extremely well and quite robustly for well over a year. You change the API, you break my code and expect me to edit tens of thousands of lines of C++ code and the deal with the potential instabilities those changes will introduce. I added #undef ENABLE_EXTRA_CHECKS to my master header file, immediately after the #include of v8.h and it has no effect. In fact, mschwartz@dionysus:~/src/SilkJS/src/v8-read-only$ grep -r ENABLE_EXTRA . mschwartz@dionysus:~/src/SilkJS/src/v8-read-only$ As a compromise, may I suggest an alternative to External::New()? How about you implement an Opaque::New() that satisfies whatever you feel necessary under the hood? On Friday, September 7, 2012 8:26:18 AM UTC-7, Jakob Kummerow wrote: > > On Fri, Sep 7, 2012 at 3:18 PM, mschwartz <[email protected] > <javascript:>>wrote: > >> I have no issue with JS being able to crash SilkJS. In fact, I see it as >> a feature. The whole point of SilkJS is to provide a C-like and low-level >> Linux/OSX programming environment using JavaScript. >> >> In C, you can crash your programs as trivially as: >> >> fwrite(buf, 10, 1, (FILE *)0xdeadbeef); >> >> The philosophy of SilkJS is that you can do likewise. However, the >> low-level bindings do return opaque handles to things as External::New() >> type things, and I implement JavaScript classes and methods that wrap the >> low-level bindings and add sanity and error checking of the arguments. >> >> Consider these C++ bindings to malloc: >> https://github.com/mschwartz/v8t/blob/master/src/binary.cpp >> vs. this JavaScript wrapper for those bindings: >> https://github.com/mschwartz/v8t/blob/master/modules/binary/index.js >> >> I fully understand the dynamic C++ class pattern >> > > Doesn't sound like you do, or at least not how it applies to the situation > at hand. Nobody is saying that you have to implement classes in C++ that > you could previously implement in JavaScript. Values created in JS and > passed into C++ land are not the issue. > > The part of the V8 API documentation that applies here is how to wrap an > External (that was created in C++!) safely before passing it into JS (where > it can't be used anyway! It can only be read from C++). And that's by > storing it as in internal field of a valid JS object, not as a property > that's accessible on the JavaScript side. Because the latter would be > inherently unsafe. > > Just because V8 in the past didn't strictly check for it doesn't mean > SilkJS (or any other embedder) should be doing this. V8's API should never > have allowed this in the first place; the fact that it did was a bug. The > question is not "do we or don't we fix this". The question is "how do we > fix this". > > >> in your link below, but I absolutely do not want to use it (for the most >> part). The cost of using it is obfuscating the implementation of such >> classes, as they need to be compiled and linked. The pattern I chose >> allows virtually all the code you'd require me to write in C++ to be coded >> in pure JavaScript, except for as thin a layer of glue functions to convert >> JS arguments to C parameters/call a library/OS function/return JS object. >> I expect people who use a server-side JavaScript platform to know >> JavaScript. I do not expect them to know C++. >> > > And yet you want the JavaScript to be able to crash just as if it were > C/C++? On a server, even? That doesn't make sense. > > >> On Friday, September 7, 2012 5:50:34 AM UTC-7, Sven Panne wrote: >>> >>> After several discussions, it is not so clear anymore what to do. First >>> of all, SilkJS does not follow https://developers.** >>> google.com/v8/embed#dynamic<https://developers.google.com/v8/embed#dynamic> >>> on >>> how to handle foreign (i.e. C/C++) pointers when embedding v8. The return >>> value of External::New is supposed to live in an internal field, but it is >>> *not* a valid JavaScript value, it is just a Foreign in disguise, sometimes >>> optimized to a Smi. Our v8.h header is very confusing regarding this fact, >>> and having External as a subclass of Value is basically wrong. Furthermore >>> Value::IsExternal is completely broken. I can see 2 ways of fixing things: >>> >>> * Keep External's implementation basically as it is. i.e. either a >>> Smi or a Foreign. If we do this, we should not keep External as a subclass >>> of Value (perhaps a subclass of Data?) and we should remove the IsExternal >>> predicate. This means that e.g. SilkJS has to change, following >>> https://developers.**google.com/v8/embed#dynamic<https://developers.google.com/v8/embed#dynamic>. >>> >>> As it is, one can easily crash SilkJS by pure JavaScript. >>> >> > I'm in favor of this. I guess we could provide a convenience function that > wraps an External as an internal field into an otherwise empty object and > returns that, but it would have to be called explicitly. > > >> >>> * Make External basically a convenience wrapper for a JavaScript >>> object with an internal property containing a Foreign. This way we could >>> keep External a subclass of value and we could fix IsExternal. The downside >>> is that all code already following https://developers.** >>> google.com/v8/embed#dynamic<https://developers.google.com/v8/embed#dynamic> >>> **would basically do a useless double indirection, punishing people >>> following that guide. >>> >> >>> We will discuss these options, there are good arguments for both of >>> them... >>> >>> Cheers, >>> S. >>> >> -- >> v8-users mailing list >> [email protected] <javascript:> >> http://groups.google.com/group/v8-users >> > > -- v8-users mailing list [email protected] http://groups.google.com/group/v8-users
