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.

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

Feat/improvements in sockets #604

Open
wants to merge 22 commits into
base: pharo-12
Choose a base branch
from

Conversation

tesonep
Copy link
Collaborator

@tesonep tesonep commented May 16, 2023

No description provided.

@daniels220
Copy link

I can't fully follow most of this, but thought I'd point something out. I noticed in digging around to deal with pharo#11723 that there already exist separate primSocket:bindTo:port: and primSocket:listenWithBacklog: which together comprise the functionality of primSocket:listenOn:backlogSize:interface:. So it's entirely possible to remove the whole primSocket:listenOn:backlogSize:interface: family of primitives altogether, and then implement:

Socket>>listenOn: portNumber backlogSize: backlog interface: ifAddr

	self
		bindTo: ifAddr port: portNumber;
		listenWithBacklog: backlog

The default values for parameters can be provided by the image as well:

listenOn: portNumber backlogSize: backlog
	self listenOn: portNumber backlogSize: backlog interface: SocketAddress zero

listenOn: port
	self listenOn: port backlogSize: 1

I think this is cleaner in a few ways—it keeps more behavior in the image, where it's easy for a user not familiar with the VM to understand and modify. It also hews closer to the call sequence that people with sockets programming experience outside of Smalltalk would recognize. I'd be happy to create the PR for updating the image code if you'd like to incorporate this into your changes.

@guillep
Copy link
Member

guillep commented Sep 4, 2023

I think this is cleaner in a few ways—it keeps more behavior in the image, where it's easy for a user not familiar with the VM to understand and modify. It also hews closer to the call sequence that people with sockets programming experience outside of Smalltalk would recognize. I'd be happy to create the PR for updating the image code if you'd like to incorporate this into your changes.

Oh yes, please, I'd be happy to!

@daniels220
Copy link

Alright, see pharo#14593. The image changes don't depend on VM changes, but do leave the primSocket:listenOn:backlogSize:interface: family orphaned/unused—you can go ahead and remove them as part of your changes here.

@tesonep tesonep changed the base branch from pharo-10 to pharo-12 September 8, 2023 08:57
@guillep
Copy link
Member

guillep commented Sep 8, 2023

Something funny is happenning there:

       0x3402ad4f8 s Process>pvtSignal:list:
       0x3402ad5e8 s Process>pvtSignal:list:
       0x3402ad6d8 s Process>pvtSignal:list:
       0x3402ad7c8 s Process>pvtSignal:list:
       0x3402ad8b8 s Process>pvtSignal:list:
       0x3402ad9a8 s Process>pvtSignal:list:
       0x3402ada98 s Process>pvtSignal:list:
       0x3402adb88 s Process>pvtSignal:list:
       0x3402adc78 s Process>pvtSignal:list:
       0x3402add68 s Process>pvtSignal:list:
       0x3402ade58 s Process>pvtSignal:list:
       0x3402adf48 s Process>pvtSignal:list:
       0x28001fc78 I Process>resume 0x3400ee1d0: a(n) Process
       0x28001fcc0 I FullBlockClosure(BlockClosure)>forkAt:named: 0x3400eeaa8: a(n) FullBlockClosure
       0x28001fd28 I TCPSocketEchoTest>withTCPEchoServer: 0x10005158908: a(n) TCPSocketEchoTest
       0x28001fd80 I [] in TCPSocketEchoTest>testEcho 0x10005158908: a(n) TCPSocketEchoTest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants