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

branch plugin-rpc-v2: use an IPC server instead of a process pipe #48

Conversation

paul-marechal
Copy link

Node has a bug on Windows preventing us from using process pipes to
communicate between processes.

Instead we'll use a named pipe on Windows and a Unix Domain Socket on
Unix-like platforms.

What it does

How to test

Review checklist

Reminder for reviewers

Node has a bug on Windows preventing us from using process pipes to
communicate between processes.

Instead we'll use a named pipe on Windows and a Unix Domain Socket on
Unix-like platforms.
@paul-marechal
Copy link
Author

I could not push new commit to your branch from the PR in Theia's main repository, hence this PR...

Comment on lines +89 to +108
const onCloseEmitter = new Emitter<ChannelCloseEvent>();
const onErrorEmitter = new Emitter<Error>();
const onMessageEmitter = new Emitter<MessageProvider>();
socket.once('close', () => onCloseEmitter.fire({ reason: 'socket closed' }));
socket.on('error', error => onErrorEmitter.fire(error));
channel.onMessage(buffer => onMessageEmitter.fire(() => new Uint8ArrayReadBuffer(buffer)));
const rpc = new RPCProtocolImpl({
getWriteBuffer: () => {
const writer = new Uint8ArrayWriteBuffer();
writer.onCommit(buffer => channel.send(buffer));
return writer;
},
close: () => {
channel.dispose();
socket.destroy();
},
onClose: onCloseEmitter.event,
onError: onErrorEmitter.event,
onMessage: onMessageEmitter.event,
});
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sketchy, feel free to improve this part.

@paul-marechal
Copy link
Author

This is no longer required.

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.

1 participant