June 02, 2017
On 6/1/17 8:25 AM, Paolo Invernizzi wrote:
> On Thursday, 1 June 2017 at 10:26:24 UTC, Steven Schveighoffer wrote:
>> I can detail exactly what happened in my code -- I am accepting dates
>> from a given week from a web request. One of the dates fell outside
>> the week, and so tried to access a 7 element array with index 9.
>> Nothing corrupted memory, but the runtime corrupted my entire process,
>> forcing a shutdown.
>
> And that's a good thing! The input should be validated, especially
> because we are talking about a web request.
>
> See it like being kind with the other side of the connection, informing
> it with a clear "rejected as the date is invalid".

If only that is what happened, I would not have started this thread!

In any case, the way forward is clear -- create containers that don't throw Error, and make them easy to use.

I think I will actually publish them, because it's a very useful thing to have. You can validate your input all you want, but if you have a program bug, or there is something you didn't consider, then the entire server isn't crashed because of it. I *like* the bounds checking, I don't have to translate back to the input what it will mean for every array access in the function -- the simple check is enough.

Still good to have it auto-restart, which I will also do. But having some sort of feedback to the client, and an attempt to continue on with other unrelated requests is preferable.

-Steve
June 02, 2017
On 06/02/2017 01:26 PM, Steven Schveighoffer wrote:
> 
> If only that is what happened, I would not have started this thread!
> 
> In any case, the way forward is clear -- create containers that don't throw Error, and make them easy to use.
> 
> I think I will actually publish them, because it's a very useful thing to have. You can validate your input all you want, but if you have a program bug, or there is something you didn't consider, then the entire server isn't crashed because of it. I *like* the bounds checking, I don't have to translate back to the input what it will mean for every array access in the function -- the simple check is enough.
> 
> Still good to have it auto-restart, which I will also do. But having some sort of feedback to the client, and an attempt to continue on with other unrelated requests is preferable.
> 
> -Steve


Hi,

I think that most people agree that an out-of-bounds access is a bug that needs to be fixed, this shouldn't be an acceptable way of running the program.

The question here seems to be what to do *in the meanwhile*, and here is the problem. I can understand the position that from a theoretical point of view the process is already unsafe at this point, and that the best option is to stop (and restart if needed).

But, in the real world if I've got a (web)server that has proper isolation, I'd much rather have a server that sends back a 500 [error message] for the buggy page and keeps working otherwise, than one that is killed and has to be restarted every time a buggy page is asked.

Think that it can be a multithreaded server, and that other ongoing (and safe!) tasks might be affected, and that safe restart, even when available, often has a performance hit.

I agree that one (perhaps even the proper) way to get this is through process isolation, but this doesn't mean that the language shouldn't allow it if needed and explicitly required. There are ways for the programmer to explicitly disable most other security features (__gshared, casting away shared and immutable, @trusted code, etc.) so why not this one?

Perhaps an intermediate solution would be to offer a compiler switch that allows Errors to be safely caught (that is, they behave as exceptions). As far as I understand from reading this thread, that's already the case in debug builds, so it cannot be that bad practice, but it would be nice to have a mode that it's otherwise "release", only with this feature turned on.

Even better would be to turn on this behaviour on a per-function basis (say @throwErrors). Although perhaps that'd be promoting this behaviour a bit too much...

Anyway, just 2¢ from a half-newbie (okay, still full-newbie :) )

June 02, 2017
On 6/1/17 2:29 PM, Walter Bright wrote:
> For your date case, the date was not validated, and was fed into an
> array, where the invalid date overflowed the array bounds. The program
> was relying on the array bounds checking to validate the data.

I think it's important to state that no, I wasn't relying on array bounds checks to validate the data. I should be validating the data (and am now). What I had was a bug in my program.

What I have been saying is that in this framework, designed the way it is, there is no good reason to crash the entire process for such a bug. There are clear delineations of when the bug is in the "user code" section of vibe.d (i.e. the code in your project) and when it is in "system code" (i.e. the vibe.d framework). I want the "system code" section to continue to function when an out of bounds error happens in "user code", to give feedback to the user that no, this didn't work, there was an internal error.

Other frameworks and languages don't have this issue. An out of range error in PHP doesn't crash apache. Similarly, a segfault in a program doesn't crash the OS. This is the way I view the layer between vibe.d framework and the code that handles requests. I get that the memory is shared, and there's a greater risk of corruption affecting the framework. The right answer is to physically separate the processes, and at some point, maybe vibe can move in that direction (I outlined what I considered a good setup in another post). But a more logical separation is still possible by requiring for instance that all request handlers are @safe. Even in that case, crashing the *fiber* and not the entire process is still preferable in cases where the input isn't properly validated. Specifically, I'm talking about out-of-bounds failures, and not general asserts.

This is why I'm still moving forward with making my arrays throw Exceptions for out-of-bounds issues (and will publish the library to do so in case anyone else feels the same way).

> 2. Array bounds checking can be turned off by a compiler switch. Program
> data validation should not be silently disabled in such an unexpected
> manner.

Most of your points are based on the assumption that this was a design decision, so they aren't applicable, but on this point I wanted to say:

IMO, anything that's on the Internet should never have array bounds checking turned off. The risk is too great.

-Steve
June 02, 2017
On 6/2/17 7:55 AM, Arafel wrote:
> But, in the real world if I've got a (web)server that has proper
> isolation, I'd much rather have a server that sends back a 500 [error
> message] for the buggy page and keeps working otherwise, than one that
> is killed and has to be restarted every time a buggy page is asked.

Yes, exactly what I want.

> Perhaps an intermediate solution would be to offer a compiler switch
> that allows Errors to be safely caught (that is, they behave as
> exceptions). As far as I understand from reading this thread, that's
> already the case in debug builds, so it cannot be that bad practice, but
> it would be nice to have a mode that it's otherwise "release", only with
> this feature turned on.

I don't think this is workable, simply because of nothrow. An Error is allowed to be thrown in nothrow code, and the compiler can simultaneously assume that nothrow functions won't throw. Therefore it can legally omit the scaffolding for deallocating scope variables when an Exception is thrown (for performance reasons), and leave your program in an invalid state.

The only conclusion I can come to is that I need to write my own array types. This isn't going to be so bad as I thought, and likely will just become second nature to use them.

-Steve
June 02, 2017
On 6/1/17 8:15 PM, aberba wrote:
> On Thursday, 1 June 2017 at 10:13:25 UTC, Steven Schveighoffer wrote:
>> On 5/31/17 6:42 PM, Jonathan M Davis via Digitalmars-d wrote:
>>> On Wednesday, May 31, 2017 19:17:16 Moritz Maxeiner via Digitalmars-d
>>> wrote:
>
>>
>> It just means that D is an inferior platform for a web framework,
>> unless you use the process-per-request model so the entire thing
>> doesn't go down for one page request. But that obviously is going to
>> cause performance problems.
>>
>> Which is unfortunate, because vibe.d is a great platform for web
>> development, other than this. You could go Adam's route and just put
>> the blinders on, but I think that's not a sustainable practice.
>>
>
> I'm glad I know enough to know this is an opinion...

Don't get me wrong, I think D will be better than other frameworks for those who are willing to work with the warts. But the perception is going to be that D web frameworks are too fragile -- one miswritten handler, and your whole webserver dies. DOS attacks will be easy with D web frameworks, even if you have distributed handling.

> anyway, its better to run a vibe.d instance in something like daemonized
> package. You should also use the vibe.d error handlers.

I found the way to restart it using systemd, so that part should be handled. Now, I need to push up moving my session handling into a persistent storage (just using the memory storage for now).

-Steve
June 02, 2017
On 6/1/17 3:49 PM, Timon Gehr wrote:
> On 01.06.2017 02:57, Moritz Maxeiner wrote:
>>>
>>> Termination of what? How on earth do you determine that the scope of
>>> this "undefined state" is the program, not the machine, or the world?
>>
>> As that is the closest scope current operating systems give us to work
>> with, this is a sane default for the runtime. Nobody stops you from
>> using a different scope if you need it.
>>
>
> Yes, they would stop me from using a smaller scope. 'nothrow' functions
> are not guaranteed to be unwindable and the compiler infers 'nothrow'
> automatically. Also, null pointer dereferences do not even throw. (On
> Linux.)

By default yes, but...

https://github.com/dlang/druntime/blob/master/src/etc/linux/memoryerror.d

-Steve
June 02, 2017
On 6/1/17 10:11 PM, Laeeth Isharc wrote:
>
> Had similar problems early on.  We used supervisord to automatically
> keep a pool of vibed applications running and put nginx in front as a
> load balancer. User session info stored in redis.  And a separate
> process for data communicating with web server over nanomsg.  Zeromq is
> more mature but I found sometimes socket could get into an inconsistent
> state if servers crashed midway, and nanomsg doesn't have this problem.
> So data update either succeeds or fails but no corruption if Web server
> crashes.
>
> Maybe better ways but it seems to be okay for us.

I think at some point, if vibe.d doesn't move in this direction, you will see a popular setup that wraps vibe.d along these lines. I imagined a similar solution earlier: https://forum.dlang.org/post/ogq7nd$ccj$1@digitalmars.com

-Steve
June 02, 2017
On 06/02/2017 02:12 PM, Steven Schveighoffer wrote:
>> Perhaps an intermediate solution would be to offer a compiler switch
>> that allows Errors to be safely caught (that is, they behave as
>> exceptions). As far as I understand from reading this thread, that's
>> already the case in debug builds, so it cannot be that bad practice, but
>> it would be nice to have a mode that it's otherwise "release", only with
>> this feature turned on.
> 
> I don't think this is workable, simply because of nothrow. An Error is allowed to be thrown in nothrow code, and the compiler can simultaneously assume that nothrow functions won't throw. Therefore it can legally omit the scaffolding for deallocating scope variables when an Exception is thrown (for performance reasons), and leave your program in an invalid state.
> 

Well, as I understood from this thread this is already possible in debug mode:

> An Exception leads to unwinding&cleanup, an Error to termination (with unwinding&cleanup in debug mode for debugging purposes).

If it is indeed so, then adding a switch that only removes this optimization (from @nothrow code) but is otherwise a release version shouldn't be too hard to implement? Even if not, making @nothrow a no-op w.r.t. unwinding should still be possible and not too hard (sorry if I'm being naïve here, I don't know how hard it would be to implement, but conceptually it seems straightforward).

Of course, one must be willing to take the performance hit.
June 02, 2017
On Friday, 2 June 2017 at 12:33:17 UTC, Steven Schveighoffer wrote:
> But the perception is going to be that D web frameworks are too fragile -- one miswritten handler, and your whole webserver dies.

Correction: "vibe.d frameworks" are fragile. This isn't D specific - my cgi.d is resilient to this (and more) and has been since 2008 since it uses a process pool. Simple solution that works very well.

Might not handle 10,000 concurrent connections... but you very rarely actually have to.
June 02, 2017
On 6/2/17 9:00 AM, Arafel wrote:
> On 06/02/2017 02:12 PM, Steven Schveighoffer wrote:
>>> Perhaps an intermediate solution would be to offer a compiler switch
>>> that allows Errors to be safely caught (that is, they behave as
>>> exceptions). As far as I understand from reading this thread, that's
>>> already the case in debug builds, so it cannot be that bad practice, but
>>> it would be nice to have a mode that it's otherwise "release", only with
>>> this feature turned on.
>>
>> I don't think this is workable, simply because of nothrow. An Error is
>> allowed to be thrown in nothrow code, and the compiler can
>> simultaneously assume that nothrow functions won't throw. Therefore it
>> can legally omit the scaffolding for deallocating scope variables when
>> an Exception is thrown (for performance reasons), and leave your
>> program in an invalid state.
>>
>
> Well, as I understood from this thread this is already possible in debug
> mode:
>
>> An Exception leads to unwinding&cleanup, an Error to termination (with
>> unwinding&cleanup in debug mode for debugging purposes).
>
> If it is indeed so, then adding a switch that only removes this
> optimization (from @nothrow code) but is otherwise a release version
> shouldn't be too hard to implement? Even if not, making @nothrow a no-op
> w.r.t. unwinding should still be possible and not too hard (sorry if I'm
> being naïve here, I don't know how hard it would be to implement, but
> conceptually it seems straightforward).
>
> Of course, one must be willing to take the performance hit.

Yes, of course. This is a non-starter if you need to compile release mode (and you do, my relatively small app is 47MB in debug mode, 20MB in release mode, and I can't imagine performance doing very well).

-Steve