Add the isolate ID checks.

Helps with the correct usage of the APIs.

src/v8/inspector/mod.rs

@@ -35,7 +35,9 @@ pub mod messages;
 #[cfg(feature = "debug-server")]
 pub mod server;
 
-use super::v8_context_scope::V8ContextScope;
+use crate::v8_c_raw::bindings::{v8_InspectorGetIsolateId, ISOLATE_ID_INVALID};
+
+use super::{isolate::IsolateId, isolate_scope::V8IsolateScope, v8_context_scope::V8ContextScope};
 
 /// The debugging inspector, carefully wrapping the
 /// [`v8_inspector::Inspector`](https://chromium.googlesource.com/v8/v8/+/refs/heads/main/src/inspector)
@@ -137,14 +139,49 @@ impl Inspector {
         Self { raw }
     }
 
+    /// Returns the isolate ID of this inspector.
+    fn get_isolate_id(&self) -> Option<IsolateId> {
+        let raw_id = unsafe { v8_InspectorGetIsolateId(self.raw.as_ptr()) };
+        if raw_id == ISOLATE_ID_INVALID {
+            None
+        } else {
+            Some(raw_id.into())
+        }
+    }
+
+    /// Returns an error if the isolate id provided isn't the one
+    /// which was used to create the inspector.
+    pub(crate) fn check_isolate_id(&self, id: Option<IsolateId>) -> Result<(), std::io::Error> {
+        let id = id.ok_or(std::io::Error::new(
+            std::io::ErrorKind::InvalidInput,
+            "The isolate doesn't have an ID.",
+        ))?;
+
+        if id
+            != self
+                .get_isolate_id()
+                .expect("The inspector has a valid isolate.")
+        {
+            Err(std::io::Error::new(
+                std::io::ErrorKind::InvalidInput,
+                "The isolate passed doesn't match with the isolate used to create the inspector.",
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
     /// Dispatches the Chrome Developer Tools (CDT) protocol message.
     /// The message must be a valid stringified JSON object with no NUL
     /// symbols, and the message must be allowed by the V8 Inspector
     /// Protocol.
     pub fn dispatch_protocol_message<T: AsRef<str>>(
         &self,
         message: T,
+        isolate_scope: &V8IsolateScope<'_>,
     ) -> Result<(), std::io::Error> {
+        self.check_isolate_id(isolate_scope.isolate.get_id())?;
+
         let message = message.as_ref();
         log::trace!("Dispatching incoming message: {message}",);
 
@@ -171,7 +208,10 @@ impl Inspector {
     pub fn schedule_pause_on_next_statement<T: AsRef<str>>(
         &self,
         reason: T,
+        isolate_scope: &V8IsolateScope<'_>,
     ) -> Result<(), std::io::Error> {
+        self.check_isolate_id(isolate_scope.isolate.get_id())?;
+
         let string = std::ffi::CString::new(reason.as_ref()).map_err(|_| {
             std::io::Error::new(
                 std::io::ErrorKind::InvalidInput,
@@ -235,7 +275,6 @@ impl Drop for Inspector {
     }
 }
 
-// TODO remove and rewrite so that we don't use it.
 /// Currently, we rely on the thread-safety of V8 which is said to not
 /// exist.
 unsafe impl Sync for Inspector {}
@@ -319,12 +358,38 @@ mod tests {
 
         // Set a "good" breakpoint.
         assert!(inspector
-            .schedule_pause_on_next_statement("Test breakpoint")
+            .schedule_pause_on_next_statement("Test breakpoint", &i_scope)
             .is_ok());
 
         // Set a "bad" breakpoint.
         assert!(inspector
-            .schedule_pause_on_next_statement("Test\0breakpoint")
+            .schedule_pause_on_next_statement("Test\0breakpoint", &i_scope)
+            .is_err());
+    }
+
+    #[test]
+    fn test_isolate_id() {
+        // Initialise the V8 engine:
+        v8_init_platform(1, Some("--expose-gc")).unwrap();
+        v8_init().unwrap();
+        // Create a new isolate:
+        let isolate = isolate::V8Isolate::new();
+        // Enter the isolate created:
+        let i_scope = isolate.enter();
+        // Create a JS execution context for code invocation:""
+        let ctx = i_scope.new_context(None);
+        // Enter the created execution context for debugging:
+        let ctx_scope = ctx.enter(&i_scope);
+        // Create an inspector.
+        let inspector = Inspector::new(&ctx_scope);
+        // This passes as we check for the same isolate id as we used
+        // to create the inspector.
+        assert!(inspector.check_isolate_id(isolate.get_id()).is_ok());
+
+        let isolate_another = isolate::V8Isolate::new();
+        // This fails the check as the isolates are different.
+        assert!(inspector
+            .check_isolate_id(isolate_another.get_id())
             .is_err());
     }
 

src/v8/inspector/server.rs

@@ -170,7 +170,7 @@
 //! // To let the remote debugger operate, we need to be able to send
 //! // and receive data to and from it. This is achieved by starting the
 //! // main loop of the debugger session:
-//! assert!(debugger_session.process_messages().is_ok());
+//! assert!(debugger_session.process_messages(&i_scope).is_ok());
 //!
 //! fake_client.join();
 //!
@@ -496,7 +496,7 @@ impl DebuggerSession {
     pub fn new(
         web_socket: WebSocketServer,
         inspector: Arc<Inspector>,
-        _isolate_scope: &V8IsolateScope<'_>,
+        isolate_scope: &V8IsolateScope<'_>,
     ) -> Result<Self, std::io::Error> {
         let connection_hints = web_socket.get_connection_hints()?;
         let web_socket = Rc::new(Mutex::new(web_socket));
@@ -525,7 +525,7 @@ impl DebuggerSession {
 
             session
                 .inspector
-                .dispatch_protocol_message(&message_string)?;
+                .dispatch_protocol_message(&message_string, isolate_scope)?;
 
             if message.is_client_ready() {
                 return Ok(session);
@@ -604,35 +604,46 @@ impl DebuggerSession {
 
     /// Waits for a message to read, reads (without parsing), proccesses
     /// it and then it.
-    pub fn read_and_process_next_message(&self) -> Result<String, std::io::Error> {
+    pub fn read_and_process_next_message(
+        &self,
+        isolate_scope: &V8IsolateScope<'_>,
+    ) -> Result<String, std::io::Error> {
         let message = self.read_next_message()?;
         log::trace!("Got incoming websocket message: {message}");
-        self.inspector.dispatch_protocol_message(&message)?;
+        self.inspector
+            .dispatch_protocol_message(&message, isolate_scope)?;
         Ok(message)
     }
 
     /// Attempts to read a message from the client. If there are no
     /// messages available to read at this time, [`None`] is returned.
-    pub fn try_read_and_process_next_message(&self) -> Result<Option<String>, std::io::Error> {
+    pub fn try_read_and_process_next_message(
+        &self,
+        isolate_scope: &V8IsolateScope<'_>,
+    ) -> Result<Option<String>, std::io::Error> {
         let message = self.try_read_next_message()?;
 
         if let Some(ref message) = message {
             log::trace!(
                 "Got incoming websocket message: {message}, len={}",
                 message.len()
             );
-            self.inspector.dispatch_protocol_message(message)?;
+            self.inspector
+                .dispatch_protocol_message(message, isolate_scope)?;
         }
         Ok(message)
     }
 
     /// Reads and processes all the next messages in a loop, until
     /// the connection is dropped by the client or until an error state
     /// is reached.
-    pub fn process_messages(&self) -> Result<(), std::io::Error> {
+    pub fn process_messages(
+        &self,
+        isolate_scope: &V8IsolateScope<'_>,
+    ) -> Result<(), std::io::Error> {
         log::trace!("Inspector main loop started.");
         loop {
-            if let Err(e) = self.read_and_process_next_message() {
+            if let Err(e) = self.read_and_process_next_message(isolate_scope) {
                 if e.kind() == std::io::ErrorKind::ConnectionAborted {
                     log::trace!("Inspector main loop successfully stopped.");
                     return Ok(());
@@ -656,11 +667,11 @@ impl DebuggerSession {
     pub fn process_messages_with_timeout(
         &self,
         duration: std::time::Duration,
-        _isolate_scope: &V8IsolateScope<'_>,
+        isolate_scope: &V8IsolateScope<'_>,
     ) -> Result<bool, std::io::Error> {
         self.set_read_timeout(Some(duration))?;
 
-        if let Err(e) = self.try_read_and_process_next_message() {
+        if let Err(e) = self.try_read_and_process_next_message(isolate_scope) {
             if e.kind() == std::io::ErrorKind::ConnectionAborted {
                 Ok(true)
             } else if e.kind() == std::io::ErrorKind::WouldBlock
@@ -679,9 +690,12 @@ impl DebuggerSession {
 
     /// Schedules a pause (sets a breakpoint) for the next statement.
     /// See [`super::Inspector::schedule_pause_on_next_statement`].
-    pub fn schedule_pause_on_next_statement(&self) -> Result<(), std::io::Error> {
+    pub fn schedule_pause_on_next_statement(
+        &self,
+        isolate_scope: &V8IsolateScope<'_>,
+    ) -> Result<(), std::io::Error> {
         self.inspector
-            .schedule_pause_on_next_statement("User breakpoint.")
+            .schedule_pause_on_next_statement("User breakpoint.", isolate_scope)
     }
 
     /// Stops the debugging session if it has been established.
@@ -853,7 +867,7 @@ mod tests {
         // main loop of the debugger session:
         // debugger_session.process_messages().expect("Debugger error");
         debugger_session
-            .read_and_process_next_message()
+            .read_and_process_next_message(&isolate_scope)
             .expect("Debugger error");
         fake_client
             .join()

v8_c_api/src/v8_c_api.cpp

@@ -442,6 +442,14 @@ class v8_inspector_client_wrapper final: public v8_inspector::V8InspectorClient
         const InspectorUserDataDeleter &deleter
     );
 
+    inline uint64_t getIsolateId() const noexcept {
+        const uint64_t *id_ptr = reinterpret_cast<uint64_t*>(isolate_->GetData(ISOLATE_ID_INDEX));
+        if (!id_ptr) {
+            return ISOLATE_ID_INVALID;
+        }
+        return *id_ptr;
+    }
+
     void dispatchProtocolMessage(const v8_inspector::StringView &message_view);
     void runMessageLoopOnPause(const int contextGroupId) override;
     void quitMessageLoopOnPause() override;
@@ -642,6 +650,10 @@ void v8_InspectorSetOnWaitFrontendMessageOnPauseCallback(
     reinterpret_cast<v8_inspector_client_wrapper *>(inspector)->setOnWaitFrontendMessageOnPauseCallback(onWaitFrontendMessageOnPauseWrapper, onDelete);
 }
 
+uint64_t v8_InspectorGetIsolateId(v8_inspector_c_wrapper *inspector) {
+    return reinterpret_cast<v8_inspector_client_wrapper *>(inspector)->getIsolateId();
+}
+
 int v8_InitializePlatform(int thread_pool_size, const char *flags) {
     if (flags) {
         v8::V8::SetFlagsFromString(flags);

v8_c_api/src/v8_c_api.h

@@ -180,6 +180,9 @@ void v8_InspectorSetOnWaitFrontendMessageOnPauseCallback(
 	v8_InspectorUserDataDeleter deleter
 );
 
+/** Returns the isolate ID this inspector was created with. */
+uint64_t v8_InspectorGetIsolateId(v8_inspector_c_wrapper *inspector);
+
 /** Creates a new v8 isolate. An isolate is a v8 interpreter that responsible to run JS code.
  * Impeder may create as many isolates as wishes.
  * initial_heap_size_in_bytes - the initial isolate heap size