saghul / txiki.js

A tiny JavaScript runtime
MIT License
2.37k stars 157 forks source link

AF_UNIX missing in namespace tjs #512

Open KaruroChori opened 3 weeks ago

KaruroChori commented 3 weeks ago

Unlike other socket related constants, this is missing. In general it seems like the library is only focusing on the INET family.

Still, Unix Domain Sockets are working as expected once properly configured, so there is no specific reason to not support them I think.

Also, it would be nice to have js helpers to construct the addr structs. At the moment we are forced to manually construct the Uint8Array, which works but it is a bit inconvenient. In case, I can do it.

lal12 commented 3 weeks ago

I didn't implement a helper and the constant because it seemed more useful to use the normal streaming tjs socket for that. It was a generic implementation for any kind of sockets. E.g. I use it for raw sockets.

lal12 commented 2 weeks ago

What's your application using the PosixSocket btw?

KaruroChori commented 2 weeks ago

I am using a file via UNIX Domain Sockets in streaming mode to share information with limited overhead between local processes and with external clients via a websockets bridge. Or at least this is the objective :D. It kinda works, but I encountered https://github.com/saghul/txiki.js/issues/513

saghul commented 2 weeks ago

Any reason not to use the builtin conect https://bettercallsaghul.com/txiki.js/api/functions/global.tjs.connect.html ? The pipe transport is unix sockets.

KaruroChori commented 2 weeks ago

I am not sure I am following. The builtin connect is built around AF_INET, requiring an address and port. IPC with Unix Domain Sockets uses only a "filename" as address, The socket address type is just not the same.

To explain it better, I was forced to write something like

    //Hardcoded 1 since the symbol is not provided
    const sock = new PosixSocket(1, tjs.SOCK_STREAM, 0);
    const addr = new Uint8Array(110);
    addr[0] = 1;
    Array.from("../test-file").map(
        (letter, i) => (addr[2 + i] = letter.charCodeAt(0)),
    );

    sock.bind(addr);

It should be

    const sock = new PosixSocket(AF_UNIX, tjs.SOCK_STREAM, 0);
    const addr = somehelperfunction('../test-file')
    sock.bind(addr);
KaruroChori commented 2 weeks ago

Ah I get it, it is called pipe internally in this implementation. I had no idea that was an option, thanks

saghul commented 2 weeks ago

It's a normal Unix domain socket, represented by a file.

Abstract sockets are not currently supported.

KaruroChori commented 2 weeks ago

Yes, I was not planning on using them. Just be clear, I assume there is no reason for them to work via posix-sockets. You are just saying they are not supported by the higher level stream implementation you are providing, right?

saghul commented 2 weeks ago

It would be nice if they work in the low level, I guess @lal12 didn't implement them because it wasn't necessary at the time.

In general the high level implementation is preferred, as it's cross-platform.

KaruroChori commented 2 weeks ago

As far as I know, the abstract variant is a linux specific extension. So it is unlikely they can be made cross-platform, but I don't know enough about windows to tell.

Also, my understanding from the code is that pipes are assuming a stream-based connection. However Unix Domain Sockets would also allow for packet and sequence packet in place of SOCK_STREAM.

saghul commented 2 weeks ago

As far as I know, the abstract variant is a linux specific extension. So it is unlikely they can be made cross-platform, but I don't know enough about windows to tell.

I mean Unix sockets / windows named pipes, not the abstract variant specifically.

Also, my understanding from the code is that pipes are assuming a stream-based connection. However Unix Domain Sockets would also allow for packet and sequence packet in place of SOCK_STREAM.

Correct, I missed to point that out. The libuv API which txiki.js wraps is only for streams.

KaruroChori commented 2 weeks ago

I see. In principle, would you accept PR on what discussed so far?

More in general, is there any roadmap or policy about what is suitable for this project and what is not, aside from what reported in https://github.com/saghul/txiki.js/issues/418?

I am extending txiki quite a bit since I am adopting it as the runtime for an embedded application. A few of these features would make sense to be here in upstream, but without discussions enabled in this repo I would have to abuse pr or issues.

saghul commented 2 weeks ago

I see. In principle, would you accept PR on what discussed so far?

Sure!

More in general, is there any roadmap or policy about what is suitable for this project and what is not, aside from what reported in #418?

No, I go by feeling :-)

I am extending txiki quite a bit since I am adopting it as the runtime for an embedded application. A few of these features would make sense to be here in upstream, but without discussions enabled in this repo I would have to abuse pr or issues.

Oh, right! I just enabled discussions, thanks for the heads up! Let;s discuss each individual change, thanks for your willingness to contribute!

KaruroChori commented 2 weeks ago

No problem going by feeling, I just wanted to make sure I was not missing anything obvious as I usually do :D. Great, thank you!

saghul commented 2 days ago

I believe this one if fixed now, right?

lal12 commented 2 days ago

I believe this one if fixed now, right?

No, AF_UNIX isn't exposed if you mean that. But I think it might be useful for opening DGRAM unix sockets or abstract unix sockets. I currently just define AF_UNIX myself. But exposing it is probably more useful. Will create a PR.