Fixes 20130413

[CyberShadow]

Tonight's bunch.

96640e5 seems to have broken the examples for me.

[s-ludwig]

Regarding the automatic exit of the event loop; apart from the missing primitives such as timers or directory watchers, there is one thing that I don't yet have a conclusion for: windows. Since vibe.d is not supposed to have a notion of a window (should be the task of a separate library because of the complexity and diverse requirements), those need to be somehow registered to prevent the event loop from exiting.

As long as these issues are not resolved, I would suggest to let haveEvents always return true to not break existing applications.

[CyberShadow]

As long as these issues are not resolved, I would suggest to let haveEvents always return true to not break existing applications.

Fair enough, but with libevent out of the picture, that leaves me with no way to develop applications which exit naturally. Do you have an idea for a compromise?

Also, what's the general policy with backwards-compatibility? Is Vibe's API already frozen, as far as backwards-compatibility goes?

[CyberShadow]

Note that your suggested behavior currently goes against runEventLoop's documentation.

Personally, I would just fix the driver so it acts like it's expected of it, and note the breakage in the docs. The use case of creating windows on the same thread as the event loop is probably relatively rare, considering that Vibe is primarily a web framework. There should probably be something in the EventDriver interface to indicate that the event loop ought to keep running, even with 0 events, instead, which would also be useful if the event thread might receive a message from another thread even when it has 0 events. libevent implements this as EVLOOP_NO_EXIT_ON_EMPTY.

[CyberShadow]

I'd say this should be logDiagnostic (well... my "Running event loop..." one probably also...)

Fixed and rebased.

[s-ludwig]

Fair enough, but with libevent out of the picture, that leaves me with no way to develop applications which exit naturally. Do you have an idea for a compromise?

In that case, do you actually need to start an event loop? Things are supposed to "just work" by falling back to EventDriver.runEventLoopOnce when no event loop is running. But it is very well possible that some features in that mode of operation have issues (e.g. a TCP listener) since it is not very well tested apart from network client kind of applications.

Also, what's the general policy with backwards-compatibility? Is Vibe's API already frozen, as far as backwards-compatibility goes?

It's not frozen and keeps moving relatively fast, but I always try to follow a proper deprecation path (spanning two or three releases), where things are phased out without breaking things first. Not doing this makes things really cumbersome as there already is a non-trivial amount of applications and often there are also dependencies on particular DMD versions, which can make it impossible to use a current compiler with an old version of the library to keep non-updated applications working.

In this particular case only partially fixing the behavior means that it introduces a regression at the same time and regressions should (obviously) always be avoided.

Note that your suggested behavior currently goes against runEventLoop's documentation.

I know, but this is still better than applications that don't run at all because the event loop exits when it thinks there is nothing left, although it is in fact (such as a timer or a window).

Personally, I would just fix the driver so it acts like it's expected of it, and note the breakage in the docs. The use case of creating windows on the same thread as the event loop is probably relatively rare, considering that Vibe is primarily a web framework.

Well, for me this is actually the primary use case ;) It allows to simplify a lot of GUI related things and may even be required on WinRT (really just guessing here).

Vibe.d is not really primarily intended to be a web framework for more general purpose, it just happens that it started out with a number of helpful features in that direction (partly because a package manager was missing at the time it started and breaking it up into parts would have been very inconvenient during development).

There should probably be something in the EventDriver interface to indicate that the event loop ought to keep running, even with 0 events, instead, which would also be useful if the event thread might receive a message from another thread even when it has 0 events. libevent implements this as EVLOOP_NO_EXIT_ON_EMPTY.

If an event loop has no listeners and no tasks running, there is no way to let anything run there, so I think there is no need for an explicit flag - just keep a task running that waits for things. It will either finish during DriverCode.notifyIdle or needs to listen for some kind of event, which will in turn keep the event loop running.

[CyberShadow]

In that case, do you actually need to start an event loop? Things are supposed to "just work" by falling back to EventDriver.runEventLoopOnce when no event loop is running.

How would that work with e.g. the example I brought up earlier, with a HTTP client starting a bunch of HTTP requests, and exiting when it's done? (The DFeed mailing list archive downloader does this)

If an event loop has no listeners and no tasks running, there is no way to let anything run there, so I think there is no need for an explicit flag - just keep a task running that waits for things.

Not sure we're on the same page here. I'm referring to the situation when there is a need to wake up an event loop from another thread, however that event loop will only wake up on a network event or such. Usually, this is thus implemented using a socket pair.

I think the canonical representation in an event loop API would be a void runInMainThread(void delegate()) method. I see that Vibe has a ManualEvent interface, but its documentation states that it is for cross-fiber notification.

I'm basically suggesting to promote EVLOOP_NO_EXIT_ON_EMPTY to an element of Vibe's API.

[s-ludwig]

In that case, do you actually need to start an event loop? Things are supposed to "just work" by falling back to EventDriver.runEventLoopOnce when no event loop is running.

How would that work with e.g. the example I brought up earlier, with a HTTP client starting a bunch of HTTP requests, and exiting when it's done? (The DFeed mailing list archive downloader does this)

That would indeed be an example that would not be optimally handled, as only serialized/blocking operations are possible in the non-event-loop mode.

If an event loop has no listeners and no tasks running, there is no way to let anything run there, so I think there is no need for an explicit flag - just keep a task running that waits for things.

Not sure we're on the same page here. I'm referring to the situation when there is a need to wake up an event loop from another thread, however that event loop will only wake up on a network event or such. Usually, this is thus implemented using a socket pair.

I think the canonical representation in an event loop API would be a void runInMainThread(void delegate()) method.

I would implement this along the lines of this:

void init()
{
    Task mainthreadtask = runTask({
        while (true) {
            receive((void function() func) {
                runTask(func);
            });
        }
    });

    runWorkerTask({
        mainthreadtask.send({ logInfo("Hello, World!"); });
    });
}

No non-exit flag needed as far as I can see as receive will automatically wait on a ManualEvent.

I see that Vibe has a ManualEvent interface, but its documentation states that it is for cross-fiber notification.

The documentation is unclear there, I'll rename it to "cross-task" to make it clearer that it is not limited to the same thread. (It already says "Note: the ownership can be shared between multiple fibers and threads.", though)

PS: I was using function in the example because passing a non-shared delegate between threads is unsafe and not allowed by the send/receive API in vibe.core.concurrency.

[CyberShadow]

That would indeed be an example that would not be optimally handled, as only serialized/blocking operations are possible in the non-event-loop mode.

OK, so how should I proceed?

The documentation is unclear there, I'll rename it to "cross-task" to make it clearer that it is not limited to the same thread.

I see. By the way, I've been meaning to ask: Why is it necessary to explicitly manage event ownership? Wouldn't it be simpler to throw when waiting on an event that another task/thread is already waiting for?

[s-ludwig]

That would indeed be an example that would not be optimally handled, as only serialized/blocking operations are possible in the non-event-loop mode.

OK, so how should I proceed?

I'm not sure how exactly the code is structured, but shouldn't something like this work:

void main()
{
    runTask({
        Task[] tasks;
        foreach (i; 0 .. 10)
            tasks ~= runTask({ fetchArticles(i); });
        foreach (t; tasks)
            t.join();
        exitEventLoop();
    });
    runEventLoop();
}

?

The documentation is unclear there, I'll rename it to "cross-task" to make it clearer that it is not limited to the same thread.

I see. By the way, I've been meaning to ask: Why is it necessary to explicitly manage event ownership? Wouldn't it be simpler to throw when waiting on an event that another task/thread is already waiting for?

For Timer and ManualEvent it already works this way. TCPConnection and File require explicit ownership transfer with the main idea being to avoid undetected race conditions when two tasks operate on the same connection. Since it is usually not necessary to manually handle ownership (the connection creator automatically has it and clients using a ConnectionPool will automatically be properly acquired/released by the pool), I think it doesn't get in the way as much as it helps to detect bugs.

It also has the advantage that it is often possible to get down to the bare event level in cases where the API doesn't provide a certain feature (e.g. waiting for either data to arrive and a ManualEvent, whichever comes first) - not pretty, but at least there is a way to do it when someone really needs it.

[s-ludwig]

OT:

        foreach (i; 0 .. 10)
            tasks ~= runTask({ fetchArticles(i); });

Oh how I hate 2043! Even I still get trapped in that from time to time, but it is really difficult to teach this every language newcomer. And everyone I know has tripped over this before.

[CyberShadow]

I'm not sure how exactly the code is structured, but shouldn't something like this work:

Yes, in practice, it would work. It wouldn't scale, though, as the top-level application would need to be aware of any tasks that would be created anywhere, which might terminate after any tasks it knows. So, your position is that I should work around this problem in my application code, rather than change something in Vibe to facilitate this? I was hoping to clean up my code in the process of rewriting DFeed, and to avoid such cludges (and fix whatever's leading to them), so that I wouldn't be ashamed to show DFeed's source as an exemplary D web/network application.

For Timer and ManualEvent it already works this way. TCPConnection and File require explicit ownership transfer with the main idea being to avoid undetected race conditions when two tasks operate on the same connection. Since it is usually not necessary to manually handle ownership (the connection creator automatically has it and clients using a ConnectionPool will automatically be properly acquired/released by the pool), I think it doesn't get in the way as much as it helps to detect bugs.

OK, this I understand, I believe you'd explained this before. My question was - what's the advantage of the current model (explicit ownership management) as opposed to a simpler model which simply checks if a flag is set, then sets it, before any operation which shouldn't be done by two tasks simultaneously?

It also has the advantage that it is often possible to get down to the bare event level in cases where the API doesn't provide a certain feature (e.g. waiting for either data to arrive and a ManualEvent, whichever comes first) - not pretty, but at least there is a way to do it when someone really needs it.

Right, I guess difficulty on waiting on either of several events is another disadvantage of the fiber model compared to the delegate model, where you'd just register both event handlers to two separate delegates. Perhaps it can be implemented in the fiber model as a method call which takes an array of delegates, and throws a special exception class in all tasks other than the first to finish, which is silenced at the top level. (Silent exceptions is something I've seen in Delphi, its Abort method.)

Oh how I hate 2043! Even I still get trapped in that from time to time, but it is really difficult to teach this every language newcomer. And everyone I know has tripped over this before.

I'm actually quite sure that's not a bug. JavaScript behaves in exactly the same way. It is a very common gotcha when working with closures, though (regardless of language).

[s-ludwig]

I'm not sure how exactly the code is structured, but shouldn't something like this work:

Yes, in practice, it would work. It wouldn't scale, though, as the top-level application would need to be aware of any tasks that would be created anywhere, which might terminate after any tasks it knows. So, your position is that I should work around this problem in my application code, rather than change something in Vibe to facilitate this? I was hoping to clean up my code in the process of rewriting DFeed, and to avoid such cludges (and fix whatever's leading to them), so that I wouldn't be ashamed to show DFeed's source as an exemplary D web/network application.

I'd say there are two options:

1. Keep it like it is now until all primitives are handled and work around it for the time being
2. Add a temporary version statement that enables the partially fixed bahavior /now/, but only explicitly until everything is handled

So lets just go with 2 - it surely doesn't make sense to jump throug hoops by adding workarounds in a large scale.

For Timer and ManualEvent it already works this way. TCPConnection and File require explicit ownership transfer with the main idea being to avoid undetected race conditions when two tasks operate on the same connection. Since it is usually not necessary to manually handle ownership (the connection creator automatically has it and clients using a ConnectionPool will automatically be properly acquired/released by the pool), I think it doesn't get in the way as much as it helps to detect bugs.

OK, this I understand, I believe you'd explained this before. My question was - what's the advantage of the current model (explicit ownership management) as opposed to a simpler model which simply checks if a flag is set, then sets it, before any operation which shouldn't be done by two tasks simultaneously?

Because if you take my previous reasoning then it would be impossible to detect if the same connection is accidentially used in multiple tasks unless they actually do a concurrent operation. But more importantly, internally there needs to be an owner anyway because the right task needs to be resumed when a specific event has arrived.

It also has the advantage that it is often possible to get down to the bare event level in cases where the API doesn't provide a certain feature (e.g. waiting for either data to arrive and a ManualEvent, whichever comes first) - not pretty, but at least there is a way to do it when someone really needs it.

Right, I guess difficulty on waiting on either of several events is another disadvantage of the fiber model compared to the delegate model, where you'd just register both event handlers to two separate delegates. Perhaps it can be implemented in the fiber model as a method call which takes an array of delegates, and throws a special exception class in all tasks other than the first to finish, which is silenced at the top level. (Silent exceptions is something I've seen in Delphi, its Abort method.)

If it's about waiting for multiple tasks then there is Task.interrupt, which throws such an exception, and Task.terminate which simply doesn't resume the task ever (so probably not a good idea). But the InterruptException could probably be safely silenced.

For waiting on multiple event objects, maybe it is possible to add some kind of generic method to EventedObject that can be used as a primitive for a generic "wait for multiple objects".

Oh how I hate 2043! Even I still get trapped in that from time to time, but it is really difficult to teach this every language newcomer. And everyone I know has tripped over this before.

I'm actually quite sure that's not a bug. JavaScript behaves in exactly the same way. It is a very common gotcha when working with closures, though (regardless of language).

If JavaScript does it like this then it must be the right thing ;)

Seriously, if something causes a bug for virtually everyone who uses a closure inside of a loop then this can be considered a language bug (read: design mistake if it is not an implementation bug). The absolute only good thing about this is when you have a loop with heaps of iterations and a function taking a delegate that is not marked as scope when it should have been (i.e. causes an unintended allocation in each loop iteration). But that would be a very poor argument for making it this way. Instead the only reason why it is this way is that it was easier to implement (one function = one stack frame with no exception for heap closures). I really have absolutely no sympathy for the current behavior - no matter how many other languages made the same mistake.

[s-ludwig]

Okay, so I'll merge now, put a version() around the haveEvents implementation to make it always return true for now and then extend it in the next step to take into account all kinds of events. Thank you for starting to tackle this and of course also for the rest of the fixes, too!