-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Implement native_synchronization within dcli/dcli/lib/src/process/process #234
Conversation
tsavo-at-pieces
commented
Mar 20, 2024
- Added native_synchronization package
- Deprecated internal mailboxes.dart
- Updated everything related to Mailboxes in the process directory within the internal dcli package
- Tested ./dcli/test/src/process/process/synchronous_test.dart on MacOS (Intel & Apple Silicon), Windows, and Linux (Ubuntu)
Implement native_synchronization within dcli/dcli/lib/src/process/process
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gave things a double check and added permalink references to the relevant native_synchronization code ✅
@@ -1,12 +1,11 @@ | |||
import 'package:dcli_core/dcli_core.dart'; | |||
|
|||
import 'process/process_sync.dart'; | |||
// import 'process/process_sync.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seemed like an used import so I commented it out for now.
// we can handly binary data. | ||
final List<List<int>> stdoutLines = <List<int>>[]; | ||
final List<List<int>> stderrLines = <List<int>>[]; | ||
int get sendAddress => send.rawAddress; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are no longer necessary.
@@ -147,18 +150,17 @@ class ProcessChannel { | |||
|
|||
/// check the data has been sent to the spawned process | |||
/// before we return | |||
final response = send.takeOneMessage(); | |||
final response = send.take(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a single message from main thread.
Reference here: https://github.com/dart-lang/native_synchronization/blob/fc4ee3f3b5cdd439ed4c18f2003468ceb24e6458/lib/mailbox.dart#L90
return NativeCalls.connectToPort(msg); | ||
} | ||
|
||
/// Starts an isolate that spawns the command. | ||
void _startIsolate(ProcessSettings processSettings, ProcessChannel channel) { | ||
unawaited(Isolate.spawn((mailboxAddrs) async { | ||
unawaited(Isolate.spawn<List<Sendable<Mailbox>>>((mailboxes) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong type to accept list of Sendable i.e. send and response.
Reference here: https://github.com/dart-lang/native_synchronization/blob/fc4ee3f3b5cdd439ed4c18f2003468ceb24e6458/lib/mailbox.dart#L130
// print('isoalte has started'); | ||
|
||
/// This code runs in the isolate. | ||
final sendMailbox = Mailbox.fromAddress(mailboxAddrs[0]); | ||
final responseMailbox = Mailbox.fromAddress(mailboxAddrs[1]); | ||
final sendMailbox = mailboxes.first.materialize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-hydrate the mailboxes within the spawned isolate which was passed as a Sendable
@@ -65,7 +70,7 @@ void _startIsolate(ProcessSettings processSettings, ProcessChannel channel) { | |||
|
|||
/// The tell the sender that we got their data and | |||
/// sent it to stdin | |||
sendMailbox.respond(Uint8List.fromList([ProcessChannel.RECEIVED])); | |||
sendMailbox.put(Uint8List.fromList([ProcessChannel.RECEIVED])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Send message back to main thread with 'put(..)'
Reference Here:
https://github.com/dart-lang/native_synchronization/blob/fc4ee3f3b5cdd439ed4c18f2003468ceb24e6458/lib/mailbox.dart#L71
channel.sendAddress, | ||
channel.responseAddress, | ||
])); | ||
List<Sendable<Mailbox>>.from([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass along channel mailboxes 'asSendables'
Reference here: https://github.com/dart-lang/native_synchronization/blob/fc4ee3f3b5cdd439ed4c18f2003468ceb24e6458/lib/mailbox.dart#L130
channel.send.asSendable, | ||
channel.response.asSendable, | ||
]), | ||
debugName: 'ProcessInIsolate')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can likely remove.
@@ -20,7 +20,7 @@ class ProcessSync { | |||
|
|||
void listenStdout(void Function(List<int>) callback) { | |||
_channel.listenStdout((data) { | |||
print('processSync recieved data from channel'); | |||
// print('processSync recieved data from channel'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can comment back in if needed.