libuv is a problem

[mhermier]

Hi,

In an attempt to unite wrenalyzer and wrencli (with the aim to modernize them) I toyed a lot more with this repo, and came to the conclusion that libuv is a problem.

There are good parts with it, but there are some issues that will be hard/impossible to circumvent. Due to the API decision to froze their API for eternity (till they change their mind):

• We can't produce a proper binding. I proposed a patch to improve the situation that landed in their master branch a while ago, but because they reinforced their opinion, libuv version 2 will never probably happen. So the binding options is not possible in a near future (in a sane manner).
• TTY handling in libuv is a mess (well it is a mess on most platform, but their API is broken somehow, and since it is frozen for eternity...). Unless I missed something, I was unable to start ANSI colors on windows terminal without introducing platform code in wren (which somehow defeat the point of using libuv). Even without that, even in their latest version, there is no option to trigger ENABLE_VIRTUAL_TERMINAL_INPUT which would solve a bunch of problem like #131 and unify input handling in wrencli.

Changing ObjForeign so it so that it could have fields would helps to solve the first to some extends, but it would not solve the second point.

The biggest annoyance is that there is no real alternative solution AFAIK. We can fork libuv and fix the issues I noted, but we will end up to have to maintain it, and considering the slow pace of the wren project it doesn't seems to be viable. We can abandon it, to have our own event library, which does not seems more realistic either considering the platform differences...

Any idea/comment is welcome.

[clsource]

IMHO I think wren-cli is fine as it is with libuv because it is a project that acts as an example Wren embed.

A new wren-cli like project made from scratch or leveraging rust, zig or golang libs can be made as a way to escape libuv problems. But that would be a different project because it would not be considered an "example" project per se, more like a full featured cli tailored for production use.

But that are just my two cents.

[joshgoebel]

We can't produce a proper binding.

I'm not sure what this means, could you please clarity?

Unless I missed something, I was unable to start ANSI colors on windows terminal without introducing platform code in wren

Could you link us to your code or a more specific example of the issue? I know I've output ANSI colors easily enough myself, but I'm also not on Windows, so perhaps things are indeed badly broken over there?

Also libuv does LOT more than TTY... I'm curious about your binding issues, but if the problem was just with TTY then one could just replace the TTY pieces, still mostly using libuv for file IO, networking, and all it's other common functionality...

and considering the slow pace of the wren project

I'm certainly interested in a faster pace for wren-console (opinionated fork of wren-cli). We use it to power all the Wren tooling at Exercism - wren-cli was not sufficient for us and moves even slower than wren proper (which is rightly where most of the attention seems to go). As ever the problem is finding those interested in contributing. I also still hope both projects might work together at some point (share code, etc)... as much of the code is [currently] still portable despite the difference in vision.

I just don't think that many are using Wren on the console. ☹️ There are many great alternatives for easy scripting such as Python, Ruby, etc - all of which have huge standard libraries (by comparison to Wren)

[mhermier]

We can't produce a proper binding.

I'm not sure what this means, could you please clarity?

The major issue is that uv_req_t does not reference the loop directly despite most of its sub type referencing it. It makes the creation of a proper binding way way way more harder and complicated than it should be. The problem lies in the fact that tracking WrenHandle and the associated WrenVM requires to have to many specific cases to be handled efficiently without introducing a potential mistake/error at a cost of to much code duplication to suit particular types.

Unless I missed something, I was unable to start ANSI colors on windows terminal without introducing platform code in wren

Could you link us to your code or a more specific example of the issue? I know I've output ANSI colors easily enough myself, but I'm also not on Windows, so perhaps things are indeed badly broken over there?

Things are not badly broken, the thing is libuv is there to abstract the tty but I did not find the way so that it enable the terminal thought in code I see that it tries to enable it, but for some reasons it does not start, unless I apply the attached patch. The patch also contains some refactoring, so IO init mirror shutdown during the VM initialization (lazy initialisation does not bring much here, but complexity).

diff --git a/src/cli/vm.c b/src/cli/vm.c
index 48d2b1e..559b57a 100644
--- a/src/cli/vm.c
+++ b/src/cli/vm.c
@@ -284,6 +284,7 @@ static void initVM()
   // Initialize the event loop.
   loop = (uv_loop_t*)malloc(sizeof(uv_loop_t));
   uv_loop_init(loop);
+  ioInit();
 }

 static void freeVM()
diff --git a/src/module/io.c b/src/module/io.c
index ad025a1..191eec2 100644
--- a/src/module/io.c
+++ b/src/module/io.c
@@ -18,6 +18,7 @@ typedef struct sFileRequestData
 } FileRequestData;

 static const int stdinDescriptor = 0;
+static const int stdoutDescriptor = 1;

 // Handle to the Stat class object.
 static WrenHandle* statClass = NULL;
@@ -31,17 +32,70 @@ static WrenHandle* stdinOnData = NULL;
 // The stream used to read from stdin. Initialized on the first read.
 static uv_stream_t* stdinStream = NULL;

+// The stream used to read from stdin.
+static uv_stream_t* stdoutStream = NULL;
+
 // True if stdin has been set to raw mode.
 static bool isStdinRaw = false;

+// Sets up a stream.
+static uv_stream_t* initStream(uv_file fd)
+{
+  if (uv_guess_handle(fd) == UV_TTY)
+  {
+    // stdin is connected to a terminal.
+    uv_tty_t* handle = (uv_tty_t*)malloc(sizeof(uv_tty_t));
+    uv_tty_init(getLoop(), handle, fd, true);
+
+    return (uv_stream_t*)handle;
+  }
+  else
+  {
+    // stdin is a pipe or a file.
+    uv_pipe_t* handle = (uv_pipe_t*)malloc(sizeof(uv_pipe_t));
+    uv_pipe_init(getLoop(), handle, false);
+    uv_pipe_open(handle, fd);
+
+    return (uv_stream_t*)handle;
+  }
+}
+
+#include <io.h>
+
+void ioInit()
+{
+#if 1
+  HANDLE handle;
+  DWORD mode;
+
+  handle = (HANDLE)_get_osfhandle(stdinDescriptor);
+  GetConsoleMode(handle, &mode);
+  SetConsoleMode(handle, mode | ENABLE_VIRTUAL_TERMINAL_INPUT);
+
+  handle = (HANDLE)_get_osfhandle(stdoutDescriptor);
+  GetConsoleMode(handle, &mode);
+  SetConsoleMode(handle, mode | ENABLE_VIRTUAL_TERMINAL_PROCESSING);
+#endif
+
+  if (stdinStream == NULL) stdinStream = initStream(stdinDescriptor);
+  if (stdoutStream == NULL) stdoutStream = initStream(stdoutDescriptor);
+}
+
+static void finiStream(uv_stream_t* stream)
+{
+  if (stream == NULL) return;
+
+  uv_close((uv_handle_t*)stream, NULL);
+  free(stream);
+}
+
 // Frees all resources related to stdin.
 static void shutdownStdin()
 {
   if (stdinStream != NULL)
   {
     uv_tty_reset_mode();
-    uv_close((uv_handle_t*)stdinStream, NULL);
-    free(stdinStream);
+    finiStream(stdinStream);
     stdinStream = NULL;
   }

@@ -58,9 +112,20 @@ static void shutdownStdin()
   }
 }

+// Frees all resources related to stdin.
+static void shutdownStdout()
+{
+  if (stdoutStream != NULL)
+  {
+    finiStream(stdoutStream);
+    stdoutStream = NULL;
+  }
+}
+
 void ioShutdown()
 {
   shutdownStdin();
+  shutdownStdout();

   if (statClass != NULL)
   {
@@ -499,30 +564,6 @@ void statIsFile(WrenVM* vm)
   wrenSetSlotBool(vm, 0, S_ISREG(stat->st_mode));
 }

-// Sets up the stdin stream if not already initialized.
-static void initStdin()
-{
-  if (stdinStream == NULL)
-  {
-    if (uv_guess_handle(stdinDescriptor) == UV_TTY)
-    {
-      // stdin is connected to a terminal.
-      uv_tty_t* handle = (uv_tty_t*)malloc(sizeof(uv_tty_t));
-      uv_tty_init(getLoop(), handle, stdinDescriptor, true);
-      
-      stdinStream = (uv_stream_t*)handle;
-    }
-    else
-    {
-      // stdin is a pipe or a file.
-      uv_pipe_t* handle = (uv_pipe_t*)malloc(sizeof(uv_pipe_t));
-      uv_pipe_init(getLoop(), handle, false);
-      uv_pipe_open(handle, stdinDescriptor);
-      stdinStream = (uv_stream_t*)handle;
-    }
-  }
-}
-
 void stdinIsRaw(WrenVM* vm)
 {
   wrenSetSlotBool(vm, 0, isStdinRaw);
@@ -530,8 +571,6 @@ void stdinIsRaw(WrenVM* vm)

 void stdinIsRawSet(WrenVM* vm)
 {
-  initStdin();
-  
   isStdinRaw = wrenGetSlotBool(vm, 1);

   if (uv_guess_handle(stdinDescriptor) == UV_TTY)
@@ -548,7 +587,6 @@ void stdinIsRawSet(WrenVM* vm)

 void stdinIsTerminal(WrenVM* vm)
 {
-  initStdin();
   wrenSetSlotBool(vm, 0, uv_guess_handle(stdinDescriptor) == UV_TTY);
 }

@@ -611,7 +649,6 @@ static void stdinReadCallback(uv_stream_t* stream, ssize_t numRead,

 void stdinReadStart(WrenVM* vm)
 {
-  initStdin();
   uv_read_start(stdinStream, allocCallback, stdinReadCallback);
   // TODO: Check return.
 }
diff --git a/src/module/io.h b/src/module/io.h
index 7d84786..a668b61 100644
--- a/src/module/io.h
+++ b/src/module/io.h
@@ -3,6 +3,8 @@

 #include "wren.h"

+void ioInit();
+
 // Frees up any pending resources in use by the IO module.
 //
 // In particular, this closes down the stdin stream.

Also libuv does LOT more than TTY... I'm curious about your binding issues, but if the problem was just with TTY then one could just replace the TTY pieces, still mostly using libuv for file IO, networking, and all it's other common functionality...

Problem is that libuv works in isolation, so you can't subclass a uv_handle_t at user level. So to bring extra functionality, dirty tricks have to be used, like dispatch some behavior based on private data, meaning you can't create handles without having an extra layer between libuv and the real code...

and considering the slow pace of the wren project

I'm certainly interested in a faster pace for wren-console (opinionated fork of wren-cli). We use it to power all the Wren tooling at Exercism - wren-cli was not sufficient for us and moves even slower than wren proper (which is rightly where most of the attention seems to go). As ever the problem is finding those interested in contributing. I also still hope both projects might work together at some point (share code, etc)... as much of the code is [currently] still portable despite the difference in vision.

I just don't think that many are using Wren on the console. frowning_face There are many great alternatives for easy scripting such as Python, Ruby, etc - all of which have huge standard libraries (by comparison to Wren)

Sure, the thing is that it is a snow ball effect positively or negatively. If we put some effort on it, it can grow in usage. In the branch I hit the windows issue, I started uniting wrenanlyzer and wren-cli. I also started introducing a user-input layer, so in the future I can do something like addAction("Ctrl+C") {|self| self.copySelection() } and abstract the keyboard physical differencies.

[joshgoebel]

The problem lies in the fact that tracking WrenHandle and the associated WrenVM

Isn't there just a single loop and single WrenVM?

[joshgoebel]

you can't subclass a uv_handle_t at user level. ... you can't create handles without

Create handles from Wren? That sounds like pretty tight coupling to me... I'd hope the Wren-side API wouldn't be so closely tied to whatever the backend C implementation is... I do agree that wrapping C objects has always seemed overly-hard to me because of the lack of fields... that alone would be a huge improvement if that ever changes.

[mhermier]

The problem lies in the fact that tracking WrenHandle and the associated WrenVM

Isn't there just a single loop and single WrenVM?

I'm talking about making a proper implementation, with possibly multiple loops, and user created handles, with callback functions...

[mhermier]

you can't subclass a uv_handle_t at user level. ... you can't create handles without

Create handles from Wren? That sounds like pretty tight coupling to me... I'd hope the Wren-side API wouldn't be so closely tied to whatever the backend C implementation is... I do agree that wrapping C objects has always seemed overly-hard to me because of the lack of fields... that alone would be a huge improvement if that ever changes.

Like-wise, more proper module binding creation, so it could be somehow more reusable that what is available currently. In any cases, even with changes on wren side to improve the situation, it is still not really clear if a proper binding would be completely possible, because of lifetime control of some libuv objects.

[joshgoebel]

with possibly multiple loops

To what end? At first glance that sounds... unnecessary? One loop (plus a stronger scheduler on the Wren side) should be more than enough, or is using lots of loops a common UV pattern I'm not aware of with some big benefits? On the C side you have to call the loops one after another in any case, unless you're running them in different processes.

I think you're talking too theoretical for me... can you give a specific example or two that you'd like to accomplish? Are you talking about wrapping the low-level libuv itself in Wren? That would indeed be quite a departure from what we're doing currently I think - and also tie us a lot more closely to libuv (for better or worse).

[mhermier]

with possibly multiple loops

To what end? At first glance that sounds... unnecessary? One loop (plus a stronger scheduler on the Wren side) should be more than enough, or is using lots of loops a common UV pattern I'm not aware of with some big benefits? On the C side you have to call the loops one after another in any case, unless you're running them in different processes.

There are multiple reasons to have multiple loops, like handling user inputs differently from general IO to preserve frame-rate on a game/programe or having multiple threaded code acting on different loops (separation of responsibility) or what ever a user strategy to handle event can be. Ideally it would be nice to have only one, but there are situation where having only one is not a valid strategy.

I think you're talking too theoretical for me... can you give a specific example or two that you'd like to accomplish? Are you talking about wrapping the low-level libuv itself in Wren? That would indeed be quite a departure from what we're doing currently I think - and also tie us a lot more closely to libuv (for better or worse).

In the contrary, the idea would be to have a more proper libuv wrapper. The fact that wren-cli would use it, would be more wren side of thing than hard-coded in the client. And with the addition of an input layer, it would abstract the even-loop from the wren application even more. On the bad side, that mean it needs a more proper module system to be present, and it also means more wren-code is in action, so performance implications must be considered.

[joshgoebel]

There are multiple reasons to have multiple loops, like handling user inputs differently from general IO to preserve frame-rate on a game/programe or having multiple threaded code acting on different loops

Sure, I did acknowledge "if you're running them in different processes" (you'd have to define what you mean by thread)... multiple loops might actually be a necessity then... but otherwise you're just running single-threaded C code with a nice event system and callbacks - there is no magic or preemptive multi-tasking happening just because you have two loops..

In my opinion the CLI needs a lot of other things (better base libraries) before it needs full multi-threading/multi-process support - but I'm glad you're not thinking small. :-)

the idea would be to have a more proper libuv wrapper.

Right now we're tightly coupled at the C layer... it seems you'd just like a nicer wrapper so we could be tightly coupled to UV at the Wren layer... but to me that's still tightly coupled. ...although I agree it might be simpler to manage if the binding was higher-level, with auto-generated bindings, etc...

Ultimately though you still have the underlying C/Wren interface though, so I'm not 100% sure what you gain by adding another layer in-between - unless that layer was indeed all auto-generated.

and it also means more wren-code is in action, so performance implications must be considered.

I'm certainly in the more-Wren-is-better camp... wren-console already goes this direction by replacing much of the C code with pure Wren, so we may be in agreement there. I think pure Wren is "fast enough" for most things... though I think many Wren people (who know C quite well) would be less thrilled about "more Wren than C"...

[joshgoebel]

Changing ObjForeign so it so that it could have fields would helps

I wonder if you couldn't handle this pattern with a code generator (for now)... the pattern I've always used was to have two classes, one pure Wren to hold the fields and the other that is all foreign (and the pure Wren class holds a reference to this)... then when/if Wren finally gets the support it'd be easy enough to update the code generated to just spit out the nicer version. It's messy, but it works.