Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bind override socket.setRawOption #188

Open
yyk808 opened this issue Jun 27, 2024 · 3 comments · May be fixed by #190
Open

bind override socket.setRawOption #188

yyk808 opened this issue Jun 27, 2024 · 3 comments · May be fixed by #190
Assignees
@pajama-coder
Labels
kind:bug

Comments

@yyk808
Copy link

yyk808 commented Jun 27, 2024

TLDR

// pipy this_file.js --reuse-port

pipy().task().onStart(new Message('Turn around')).connect('127.0.0.1:8000', {
  bind: '127.0.0.1:1234',
  onState: (conn) => {
    if(conn.state === 'open') {
      conn.socket.setRawOption(1, 15, new Data([1]))
    }
  },
}).listen('127.0.0.1:1234').print()

The code above won't work correctly.

Reproduce

  1. Setting up a server returning message.
pipy -e 'pipy().listen("127.0.0.1:8000").print().connect("127.0.0.1:1234")'
  1. Using code in the tldr to get error log like:
❯ pipy error.js --reuse-port
2024-06-27 21:49:31.069 [INF] [config]
2024-06-27 21:49:31.069 [INF] [config] Module /error.js
2024-06-27 21:49:31.069 [INF] [config] ================
2024-06-27 21:49:31.069 [INF] [config]
2024-06-27 21:49:31.069 [INF] [config]  [Listen on 1234 at 127.0.0.1]
2024-06-27 21:49:31.069 [INF] [config]  ----->|
2024-06-27 21:49:31.069 [INF] [config]        |
2024-06-27 21:49:31.069 [INF] [config]       print -->|
2024-06-27 21:49:31.069 [INF] [config]                |
2024-06-27 21:49:31.069 [INF] [config]  <-------------|
2024-06-27 21:49:31.069 [INF] [config]  
2024-06-27 21:49:31.069 [INF] [config]  [Task #1 ()]
2024-06-27 21:49:31.069 [INF] [config]  ----->|
2024-06-27 21:49:31.069 [INF] [config]        |
2024-06-27 21:49:31.069 [INF] [config]       connect -->|
2024-06-27 21:49:31.069 [INF] [config]                  |
2024-06-27 21:49:31.069 [INF] [config]  <---------------|
2024-06-27 21:49:31.069 [INF] [config]  
2024-06-27 21:49:31.069 [INF] [listener] Listening on TCP port 1234 at 127.0.0.1
2024-06-27 21:49:31.069 [ERR] connect() at line 1 column 58 in /error.js: bind: Address already in use
2024-06-27 21:49:31.069 [ERR] connect() at line 1 column 58 in /error.js: bind: Address already in use
2024-06-27 21:49:31.069 [ERR] connect() at line 1 column 58 in /error.js: bind: Address already in use
^C2024-06-27 21:49:32.175 [INF] [shutdown] Shutting down...
2024-06-27 21:49:32.175 [INF] [shutdown] Waiting for workers to drain...
2024-06-27 21:49:32.175 [INF] [listener] Stopped listening on TCP port 1234 at 127.0.0.1
  1. Also, I've made a python equivalent. If everything works fine in pipy, the pjs code should act just like this.
import socket

server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)

client = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
client.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)

BIND = 1234
TARGET = 8000

try:
    client.bind(('127.0.0.1', BIND))
    server.bind(('127.0.0.1', BIND))

    server.listen(1)
    client.connect(('127.0.0.1', TARGET))

    client.sendall(b'Turn around')

    s, _ = server.accept()
    data = s.recv(16)
    s.close()

    print(data.decode())

finally:
    client.close()
    server.close()

Reason

I tried to add a print in src/socket.cpp line 885 where we define setRawOption, it will only be called when not set bind option.
So it comes out that bind will override setRawOption.
@pajama-coder
Copy link
Collaborator

@yyk808 Yes, the logic of connect() filter wasn't quite right, it didn't actually open the socket by the time when onState is called with 'open' state. I've fixed that in the latest commit, where onState happens after the socket is open and before calling bind(). That should be the right time for you to do setsockopt().

As for the "Address already in use" errors, that's just normal since reuse-port is only meaningful for server-side sockets. I don't think that's a problem for pipy.

@keveinliu
Copy link
Collaborator

@yyk808 Fixed in commit ad5a517

@yyk808
Copy link
Author

yyk808 commented Jul 4, 2024 •
edited
Loading

@yyk808 Yes, the logic of connect() filter wasn't quite right, it didn't actually open the socket by the time when onState is called with 'open' state. I've fixed that in the latest commit, where onState happens after the socket is open and before calling bind(). That should be the right time for you to do setsockopt().

As for the "Address already in use" errors, that's just normal since reuse-port is only meaningful for server-side sockets. I don't think that's a problem for pipy.

ad5a517 fixed state error, so the setsockopt() will get the socket native handle correctly.

I'm convinced that "Address already in use" is indeed a bug because the setsockopt() is still not working correctly. If we print out the return value of this function, we will find that the errno is still not 0 but 22.

#define	EINVAL		22	/* Invalid argument */
pipy().task().onStart(new Message('Turn around')).connect('127.0.0.1:8000', {
	onState: (conn) => {
		if(conn.state === 'open') {
			console.info(conn.socket.setRawOption(1, 15, new Data([1])))
		}
	},
	bind: '127.0.0.1:1234'
})

截图 2024-07-04 22-16-14

@yyk808 yyk808 linked a pull request Jul 4, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pajama-coder pajama-coder

Labels
kind:bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[fix] setsockopt errno 22 yyk808/pipy
3 participants
@keveinliu @yyk808 @pajama-coder