WIP: Routine Unresponsive due to Deadlock #51

Draft
modulatingforce wants to merge 10 commits from issue-routine-lock into routines-functionality

Initial Description

Stemming from feature set from Routines Functionality - #40 (comment)

ISSUE . The defined Routine's Custom Execution Body is being reached (so the custom fn is being triggered as expected); However, I believe at L413 where c.read().await.channel , it never goes beyond that point within the same Custom fn body

            if let BotAction::R(c) = &*guard {
                println!("{:?}",c.read().await.channel);
            }
            println!("Critical code area end"); // <= 03.29 - ISSUE This is NOT printed 

Even if the bot continues to be responsive (i.e., the main bot thread continues to run), the rest of the routine is not. I believe this is suggesting there's a lock with one of the objects we're introducing with this feature . In the above line, I believe the lock involves ExecBodyParam's curr_act value

I believe the issue is a deadlock between custom and core locks : However, ideally this is something that can be identified or some best practices recommended, even if we do identify that something in the custom module is the issue

High Priority . This is a blocker of Routines functionality


Additional Resources


Plan of Action

  • Flesh out Issue/Requirements Description > @modulatingforce - Skipping
  • Review Code for Recommendations regarding locks
    • Routines Functional Code Workflow > @modulatingforce - Skipping
    • In particular, identify where read and write locks are located, and ensure they're dropped (e.g., within a block or through explicit drop)'
    • See if there's opportunities to reduce the number of guards
  • Code Recommended Solution that avoids deadlocks & allow self refencing
  • Test Solution
  • If changes were required in custom module, consider creating SOP recommendations for Custom developers
  • Unit Tests? Probably not feasible but depends on solution.
# Initial Description Stemming from feature set from Routines Functionality - https://git.flake.sh/modulatingforce/forcebot_rs/pulls/40#issuecomment-924 **_ISSUE ._** The defined Routine's Custom Execution Body is being reached (so the custom `fn` is being triggered as expected); However, I believe at L413 where `c.read().await.channel` , it never goes beyond that point within the same Custom `fn` body ```rust if let BotAction::R(c) = &*guard { println!("{:?}",c.read().await.channel); } println!("Critical code area end"); // <= 03.29 - ISSUE This is NOT printed ``` Even if the bot continues to be responsive (i.e., the `main` bot thread continues to run), the rest of the routine is not. I *believe* this is suggesting there's a lock with one of the objects we're introducing with this feature . In the above line, I believe the lock involves `ExecBodyParam`'s `curr_act` value I believe the issue is a deadlock between `custom` and `core` locks : However, ideally this is something that can be identified or some best practices recommended, even if we do identify that something in the custom module is the issue High Priority . This is a blocker of Routines functionality --- # Additional Resources - `tokio::RwLock` - https://docs.rs/tokio/latest/tokio/sync/struct.RwLock.html - Blog Post on an example Deadlock in Rust - https://www.snoyman.com/blog/2024/01/best-worst-deadlock-rust/ --- # Plan of Action - [x] Flesh out Issue/Requirements Description > @modulatingforce - Skipping - [x] Review Code for Recommendations regarding locks - [x] Routines Functional Code Workflow > @modulatingforce - Skipping - [x] In particular, identify where `read` and `write` locks are located, and ensure they're dropped (e.g., within a block or through explicit `drop`)' - [x] See if there's opportunities to reduce the number of guards - [x] Code Recommended Solution that avoids deadlocks & allow self refencing - [ ] Test Solution - [ ] If changes were required in `custom` module, consider creating SOP recommendations for Custom developers - [ ] Unit Tests? Probably not feasible but depends on solution.
modulatingforce added this to the Prototype 1.0 milestone 2024-03-29 09:37:08 -04:00
modulatingforce added this to the Forcebot Prototype 1.0 Push project 2024-03-29 09:37:09 -04:00
modulatingforce added a new dependency 2024-03-29 09:38:49 -04:00
Author
Owner

Hmm. I'm wondering if I can delegate this to someone like tori or if this is a lot of manual work / investigation

If tori is willing to pick it up after reading the initial description, I may need to adjust the description with some additional details like Functional Use Case (that might not be as clear, since we're just introducing routines on parent PR)

  • Will be reaching out to Tori if he's willing to collaborate
Hmm. I'm wondering if I can delegate this to someone like tori or if this is a lot of manual work / investigation If tori is willing to pick it up after reading the initial description, I may need to adjust the description with some additional details like Functional Use Case (that might not be as clear, since we're just introducing routines on parent PR) - [x] Will be reaching out to Tori if he's willing to collaborate
mzntori added 2 commits 2024-03-29 16:08:12 -04:00
Collaborator

The Routine that you want to read lock in the execution body is the routine that is used to call that execution body, making it so the execution body waits for itself to end. No idea how you would do this, but I suspect that you cant do that. If you want to access the channel you probably have to pass it in a different way instead of just passing the routine itself.

The Routine that you want to read lock in the execution body is the routine that is used to call that execution body, making it so the execution body waits for itself to end. No idea how you would do this, but I suspect that you cant do that. If you want to access the channel you probably have to pass it in a different way instead of just passing the routine itself.
Collaborator

Second commit isn't necessary btw you can roll that back if you want, but it just has a bunch of debug implementations that might help debugging stuff like this in the future.

Second commit isn't necessary btw you can roll that back if you want, but it just has a bunch of debug implementations that might help debugging stuff like this in the future.
modulatingforce self-assigned this 2024-03-30 10:57:05 -04:00
mzntori was assigned by modulatingforce 2024-03-30 10:57:05 -04:00
modulatingforce reviewed 2024-03-30 11:02:51 -04:00
modulatingforce left a comment
Author
Owner

Wow @mzntori you are beautiful peepoShy

std::fmt::{Debug, Formatter}; and dbg! are a learning experience for me! Good share!

uuh feel free to resolve this comment afterwards? lol testing this review process as well

  • Feel free to resolve the below comments after you read or whatever?
Wow @mzntori you are beautiful peepoShy `std::fmt::{Debug, Formatter};` and `dbg!` are a learning experience for me! Good share! uuh feel free to resolve this comment afterwards? lol testing this review process as well - [ ] Feel free to resolve the below comments after you read or whatever?
@ -535,0 +538,4 @@
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "{:?} {:?} {:?} {:?} {:?}", self.module, self.command, self.alias, self.help, self.required_roles)
}
}
Author
Owner

Holy I love this idea! 🤤 with the others too!

Holy I love this idea! 🤤 with the others too!
@ -1228,2 +1263,3 @@
).await;
dbg!("after");
Author
Owner

wow! dbg! is so useful! Good share!

wow! `dbg!` is so useful! Good share!
modulatingforce reviewed 2024-03-30 11:29:31 -04:00
modulatingforce left a comment
Author
Owner

@mzntori Definitely agree that you're in the right area , and I think it's related . However, what's strange is that this exec_body is called and executed - if the lock occurs at this spot, I don't believe I would see exec_body internal code start to execute . This may be related though

Few additional Comments :

  • It's within the exec_body body that the lock occurs 😢
  • within the defined exec_body, if you remove code related to locks, the Routine Works without issues and triggers the rest of it's loop without issues
    • For example, if a custom module developer defined the custom fn associated with exec_body to just println! , the Routine goes across without issues
  • In this same area withint exec_body , if you continue to add dbg! around there, you'll see it get around locks

😓I know this is a bit complex, so I'll writeup soon-ish the observed workflow that might clarify this

@mzntori Definitely agree that you're in the right area , and I think it's related . However, what's strange is that this `exec_body` is called and executed - if the lock occurs at this spot, I don't believe I would see `exec_body` internal code start to execute . This may be related though Few additional Comments : - It's within the `exec_body` body that the lock occurs 😢 - _within_ the defined `exec_body`, if you remove code related to locks, the `Routine` _Works without issues_ and triggers the rest of it's loop without issues - For example, if a `custom` module developer defined the custom `fn` associated with `exec_body` to just `println!` , the Routine goes across without issues - In this same area withint `exec_body` , if you continue to add `dbg!` around there, you'll see it get around locks 😓I know this is a bit complex, so I'll writeup soon-ish the observed workflow that might clarify this
@ -1226,3 +1260,4 @@
// So execbody waits for itself to finish.
(self_ar.read().await.exec_body)(
self_ar.read().await.parent_params.clone()
).await;
Author
Owner

Just for added clarity, at L1262 self_ar.read().await.parent_params.clone() :

  • This is within BotModules impl Routine method async fn loopbody(&mut self)
Just for added clarity, at L1262 `self_ar.read().await.parent_params.clone()` : - This is within `BotModules` `impl Routine` method `async fn loopbody(&mut self)`
modulatingforce added
Phase 1.0
Requirements > Researching
and removed
Phase 1.0
Requirements > Drafting
labels 2024-03-30 11:29:59 -04:00
Author
Owner

Useful Tool - tokio-console crate for Diagnosing tokio

Learned about a useful tool tokio-console that could be used to troubleshoot async issues, including possibly Deadlock issues

I figured as code is getting more complex, it will be more difficult to track tokio async deadlocks . This will ease identifying these issues earlier I think

Relatively easy to setup but very powerful to use

I'm sure @notohh will love this 😆it screams devops to me for some reason

Below is an example where I ran it and identified which RwLock were observed while the Bot is current running, and information like number of current_readers for that RwLock

image

I've added it in a commit I'll be adding shortly, but it did adjust the Cargo.lock by a lot - I may need someone to just see if there's concerns or not

For More Information

# Useful Tool - `tokio-console` crate for Diagnosing `tokio` Learned about a useful tool `tokio-console` that could be used to troubleshoot async issues, including possibly Deadlock issues I figured as code is getting more complex, it will be more difficult to track `tokio` async deadlocks . This will ease identifying these issues earlier I think Relatively easy to setup but very powerful to use I'm sure @notohh will love this 😆it screams devops to me for some reason Below is an example where I ran it and identified which `RwLock` were observed while the Bot is current running, and information like number of `current_readers` for that `RwLock` ![image](/attachments/1a126617-03f9-458c-a794-1ef5735a27ec) I've added it in a commit I'll be adding shortly, but it did adjust the Cargo.lock by a lot - I may need someone to just see if there's concerns or not ## For More Information - `tokio-rs` crate - [github](https://github.com/tokio-rs/console) - [crates.io](https://crates.io/crates/tokio-console) - [docs.rs](https://docs.rs/tokio-console/latest/tokio_console/) - `conscole-subscriber` crate - [crates.io](https://crates.io/crates/console-subscriber)
201 KiB
modulatingforce added 3 commits 2024-03-30 14:26:55 -04:00
Author
Owner

Update - Related to this PR / Issue

  • To hopefully simplify and isolate the issue/crucial code, I've made the Custom Child fn that the Routine Calls call ExecBody::get_channel()

    • Within this get_channel() call, an unexpected deadlock is occurring
  • At the moment, tokio-console doesn't add to what we know already : in particular, the issue has to do with the Arc<RwLock<Routine>> & Arc<RwLock<BotAction>>

  • with tokio-console tool and CodeLLB extension, I was able to add more debugging on related RwLocks and their current_reader in critical parts of code

  • I did the following to address the issue , but didn't seem to help - but will be keeping these in place just in case (I think these are better in code)

    • Added additional explicit drops and let for RwLock Guards
  • I suspect it has something to do with the relationship of BotAction and Routine and how they store their own self references

    • However, my initial tests at the moment in RustPlayground don't reproduce the lock when I recreate that relatoinship . * I still need to continue coding to match the problem scenario fully though
**Update - Related to this PR / Issue** - To hopefully simplify and isolate the issue/crucial code, I've made the Custom Child `fn` that the Routine Calls call `ExecBody::get_channel()` - Within this `get_channel()` call, an unexpected deadlock is occurring - At the moment, `tokio-console` doesn't add to what we know already : in particular, the issue has to do with the `Arc<RwLock<Routine>>` & `Arc<RwLock<BotAction>>` - with `tokio-console` tool and `CodeLLB` extension, I was able to add more debugging on related `RwLock`s and their `current_reader` in critical parts of code - I did the following to address the issue , but didn't seem to help - but will be keeping these in place just in case (I think these are better in code) - Added additional explicit `drops` and `let` for `RwLock` Guards - I _suspect_ it has something to do with the relationship of `BotAction` and `Routine` and how they store their own self references - However, my initial tests at the moment in RustPlayground don't reproduce the lock when I recreate that relatoinship . * I still need to continue coding to match the problem scenario fully though
modulatingforce reviewed 2024-03-30 14:34:11 -04:00
modulatingforce left a comment
Author
Owner
  • Need to double check if tokio-console and console-subscriber crates need to be adjusted or no issues
- [ ] Need to double check if `tokio-console` and `console-subscriber` crates need to be adjusted or no issues
@ -16,2 +16,4 @@
casual_logger = "0.6.5"
chrono = "0.4.35"
tokio-console = "0.1.10"
console-subscriber = "0.2.0"
Author
Owner

@notohh & @mzntori - these two crates I added tokio-console and console-subscriber created very large changes in Cargo.lock . Do you see this as an issue?

I'm going to shrug it off cuz LULE I don't know - but this is really helpful

If you're not sure or you may need time to look, I'll just shrug this off for now and say let's add

More information on these crates:

@notohh & @mzntori - these two crates I added `tokio-console` and `console-subscriber` created very large changes in `Cargo.lock` . **Do you see this as an issue?** I'm going to shrug it off cuz LULE I don't know - but this is really helpful If you're not sure or you may need time to look, I'll just shrug this off for now and say let's add More information on these crates: - https://git.flake.sh/modulatingforce/forcebot_rs/pulls/51#issuecomment-973
modulatingforce reviewed 2024-03-30 14:42:28 -04:00
modulatingforce left a comment
Author
Owner

Notes on the Identified Critical Areas

Notes on the Identified Critical Areas
@ -78,0 +80,4 @@
let out = Some(r.read().await.channel.clone());
// => 03.30 - This isn't reached because of the Deadlock
dbg!(">> Just after Potential Deadlock Lock");
Author
Owner

L74-L83 Observations at this Critical Deadlock

                dbg!("Core > ExecBodyParams > GetChannels - routine identified");
                dbg!(">> BotActionAR - RwLock from botmodules.rs::929:46 - current_readers = 1 ");
                dbg!(">> RoutineAR - RwLock from botmodules.rs::1261:32 - current_readers = 1");
                dbg!(">> join_handle - RwLock from botmodules.rs::1226:46 - current_readers = 0");
                dbg!(">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 3 ");
                // => 03.30 - Just before deadlock
                let out = Some(r.read().await.channel.clone());

                // => 03.30 - This isn't reached because of the Deadlock
                dbg!(">> Just after Potential Deadlock Lock");
L74-L83 Observations at this Critical Deadlock ```rust dbg!("Core > ExecBodyParams > GetChannels - routine identified"); dbg!(">> BotActionAR - RwLock from botmodules.rs::929:46 - current_readers = 1 "); dbg!(">> RoutineAR - RwLock from botmodules.rs::1261:32 - current_readers = 1"); dbg!(">> join_handle - RwLock from botmodules.rs::1226:46 - current_readers = 0"); dbg!(">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 3 "); // => 03.30 - Just before deadlock let out = Some(r.read().await.channel.clone()); // => 03.30 - This isn't reached because of the Deadlock dbg!(">> Just after Potential Deadlock Lock"); ```
@ -1230,0 +1293,4 @@
{
let guard2 = self_ar.read().await;
(guard2.exec_body)(parent_params).await;
drop(guard2);
Author
Owner

L1288-L1296 - Observations during this line - prior to Critical Deadlock

	    dbg!("Core > Guarding & Executing Child Execution Body");
        dbg!(">> BotActionAR - RwLock from botmodules.rs::929:46 - current_readers = 0 ");
        dbg!(">> RoutineAR - RwLock from botmodules.rs::1261:32 - current_readers = Not Listed");
        dbg!(">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 3 ");

        {
            let guard2 = self_ar.read().await;
            (guard2.exec_body)(parent_params).await;
            drop(guard2);
L1288-L1296 - Observations during this line - prior to Critical Deadlock ```rust dbg!("Core > Guarding & Executing Child Execution Body"); dbg!(">> BotActionAR - RwLock from botmodules.rs::929:46 - current_readers = 0 "); dbg!(">> RoutineAR - RwLock from botmodules.rs::1261:32 - current_readers = Not Listed"); dbg!(">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 3 "); { let guard2 = self_ar.read().await; (guard2.exec_body)(parent_params).await; drop(guard2); ```
Author
Owner

The Routine that you want to read lock in the execution body is the routine that is used to call that execution body, making it so the execution body waits for itself to end. No idea how you would do this, but I suspect that you cant do that. If you want to access the channel you probably have to pass it in a different way instead of just passing the routine itself.

To be clear though, I do agree with this : this does merit exploration if we can't self pass routine . If push comes to shove, we can prioritize exploring a different way to retrieve the desired values without self reference (channel was chosen here for a simplified case)

::Giggles:: but my gut feeling says this is possible, like how BotInstance is now able to pass itself into commands

  • though understanding,BotInsance doesn't use self referencing atm so might make sense
> The Routine that you want to read lock in the execution body is the routine that is used to call that execution body, making it so the execution body waits for itself to end. No idea how you would do this, but I suspect that you cant do that. If you want to access the channel you probably have to pass it in a different way instead of just passing the routine itself. To be clear though, I do agree with this : **this does merit exploration if we can't self pass routine** . If push comes to shove, we can prioritize exploring a different way to retrieve the desired values without self reference (`channel` was chosen here for a simplified case) ::Giggles:: but my gut feeling says this is possible, like how `BotInstance` is now able to pass itself into commands - though understanding,`BotInsance` doesn't use self referencing atm so might make sense
modulatingforce added the
Ownership
Collab
label 2024-03-31 09:46:16 -04:00
modulatingforce added 3 commits 2024-03-31 16:38:21 -04:00
modulatingforce reviewed 2024-03-31 18:24:06 -04:00
modulatingforce left a comment
Author
Owner

Confirmed Fix

Code Fix Summary

  • For Operations that are likely short or should be considered a single unit, consider using Blocking methods instead
  • For Routine, we had code fixes involving using blocking methods for Routine functionality
    • For others, we will review separately in #54

Code Fix Involved :

  • Within Blocking Non-Async functions, when accessing Arc<RwLock<BotAction>> , use blocking_read() & blocking_write()
  • ExecBodyParams::get_channel() be defined as non-async
  • Routine definition involve :
    • exec_body : fn(Arc<RwLock<ExecBodyParams>>)
    • parent_params : Arc<RwLock<ExecBodyParams>>
  • Routine::start() involves a tokio::task::spawn_blocking execute the loopbody()
### Confirmed Fix ### Code Fix Summary - For Operations that are likely short or should be considered a single unit, consider using Blocking methods instead - For `Routine`, we had code fixes involving using blocking methods for `Routine` functionality - For others, we will review separately in https://git.flake.sh/modulatingforce/forcebot_rs/issues/54 ### Code Fix Involved : - [x] Within Blocking Non-Async functions, when accessing `Arc<RwLock<BotAction>>` , use `blocking_read()` & `blocking_write()` - [x] `ExecBodyParams::get_channel()` be defined as non-async - [x] `Routine` definition involve : - [x] `exec_body : fn(Arc<RwLock<ExecBodyParams>>)` - [x] `parent_params : Arc<RwLock<ExecBodyParams>>` - [x] `Routine::start()` involves a `tokio::task::spawn_blocking` execute the `loopbody()`
@ -52,2 +52,3 @@
pub async fn get_channel(&self) -> Option<Channel> {
// pub async fn get_channel(&self) -> Option<Channel> {
pub fn get_channel(&self) -> Option<Channel> {
Author
Owner

Code Fix Involved :

  • Within Blocking Non-Async functions, when accessing Arc<RwLock<BotAction>> , use blocking_read() & blocking_write()
  • ExecBodyParams::get_channel() be defined as non-async
  • Routine definition involve :
    • exec_body : fn(Arc<RwLock<ExecBodyParams>>)
    • parent_params : Arc<RwLock<ExecBodyParams>>
  • Routine::start() involves a tokio::task::spawn_blocking execute the loopbody()
### Code Fix Involved : - [x] Within Blocking Non-Async functions, when accessing `Arc<RwLock<BotAction>>` , use `blocking_read()` & `blocking_write()` - [x] `ExecBodyParams::get_channel()` be defined as non-async - [x] `Routine` definition involve : - [x] `exec_body : fn(Arc<RwLock<ExecBodyParams>>)` - [x] `parent_params : Arc<RwLock<ExecBodyParams>>` - [x] `Routine::start()` involves a `tokio::task::spawn_blocking` execute the `loopbody()`
@ -63,0 +70,4 @@
thread 'tokio-runtime-worker' panicked at src\core\bot_actions.rs:66:38:
Cannot block the current thread from within a runtime. This happens because a function attempted to block the current thread while the thread is being used to drive asynchronous tasks.
*/
let curr_act_lock = curr_act.blocking_read();
Author
Owner

Code Fix Involved :

  • Within Blocking Non-Async functions, when accessing Arc<RwLock<BotAction>> , use blocking_read() & blocking_write()
  • ExecBodyParams::get_channel() be defined as non-async
  • Routine definition involve :
    • exec_body : fn(Arc<RwLock<ExecBodyParams>>)
    • parent_params : Arc<RwLock<ExecBodyParams>>
  • Routine::start() involves a tokio::task::spawn_blocking execute the loopbody()
### Code Fix Involved : - [x] Within Blocking Non-Async functions, when accessing `Arc<RwLock<BotAction>>` , use `blocking_read()` & `blocking_write()` - [x] `ExecBodyParams::get_channel()` be defined as non-async - [x] `Routine` definition involve : - [x] `exec_body : fn(Arc<RwLock<ExecBodyParams>>)` - [x] `parent_params : Arc<RwLock<ExecBodyParams>>` - [x] `Routine::start()` involves a `tokio::task::spawn_blocking` execute the `loopbody()`
@ -634,0 +648,4 @@
// exec_body : fn(ExecBodyParams),
exec_body : fn(Arc<RwLock<ExecBodyParams>>),
// pub parent_params : ExecBodyParams ,
pub parent_params : Arc<RwLock<ExecBodyParams>> ,
Author
Owner

Code Fix Involved :

  • Within Blocking Non-Async functions, when accessing Arc<RwLock<BotAction>> , use blocking_read() & blocking_write()
  • ExecBodyParams::get_channel() be defined as non-async
  • Routine definition involve :
    • exec_body : fn(Arc<RwLock<ExecBodyParams>>)
    • parent_params : Arc<RwLock<ExecBodyParams>>
  • Routine::start() involves a tokio::task::spawn_blocking execute the loopbody()
### Code Fix Involved : - [x] Within Blocking Non-Async functions, when accessing `Arc<RwLock<BotAction>>` , use `blocking_read()` & `blocking_write()` - [x] `ExecBodyParams::get_channel()` be defined as non-async - [x] `Routine` definition involve : - [x] `exec_body : fn(Arc<RwLock<ExecBodyParams>>)` - [x] `parent_params : Arc<RwLock<ExecBodyParams>>` - [x] `Routine::start()` involves a `tokio::task::spawn_blocking` execute the `loopbody()`
@ -1109,0 +1157,4 @@
drop(guard1);
});
loopbodyspawn.await.unwrap();
Author
Owner

Code Fix Involved :

  • Within Blocking Non-Async functions, when accessing Arc<RwLock<BotAction>> , use blocking_read() & blocking_write()
  • ExecBodyParams::get_channel() be defined as non-async
  • Routine definition involve :
    • exec_body : fn(Arc<RwLock<ExecBodyParams>>)
    • parent_params : Arc<RwLock<ExecBodyParams>>
  • Routine::start() involves a tokio::task::spawn_blocking execute the loopbody()
### Code Fix Involved : - [x] Within Blocking Non-Async functions, when accessing `Arc<RwLock<BotAction>>` , use `blocking_read()` & `blocking_write()` - [x] `ExecBodyParams::get_channel()` be defined as non-async - [x] `Routine` definition involve : - [x] `exec_body : fn(Arc<RwLock<ExecBodyParams>>)` - [x] `parent_params : Arc<RwLock<ExecBodyParams>>` - [x] `Routine::start()` involves a `tokio::task::spawn_blocking` execute the `loopbody()`
@ -1152,6 +1209,8 @@ impl Routine {
}
dbg!("SUCCESS! Routinecompleted");
Author
Owner

Resolution Confirmed

In stdout ,

  • this line is reached
  • the inner fn of the custom child body is reached
[src\core\bot_actions.rs:89] ">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 2" = ">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 2"
[src\custom\experimental\experiment003.rs:476] "Custom > within Child Custom fn - after GetChannel" = "Custom > within Child Custom fn - after GetChannel"
Some(Channel("modulatingforcebot"))
Critical code area end
[src\custom\experimental\experiment003.rs:481] "Custom > within Child Custom fn - Before Critical area" = "Custom > within Child Custom fn - Before Critical area"
[src\core\botmodules.rs:1366] "Core > After Execution Body is completed" = "Core > After Execution Body is completed"
[src\core\botmodules.rs:1212] "SUCCESS! Routinecompleted" = "SUCCESS! Routinecompleted"
### Resolution Confirmed In `stdout` , - this line is reached - the inner `fn` of the custom child body is reached ```powershell [src\core\bot_actions.rs:89] ">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 2" = ">> BotInstanceAR - RwLock from botinstance.rs::150:28 - current_readers = 2" [src\custom\experimental\experiment003.rs:476] "Custom > within Child Custom fn - after GetChannel" = "Custom > within Child Custom fn - after GetChannel" Some(Channel("modulatingforcebot")) Critical code area end [src\custom\experimental\experiment003.rs:481] "Custom > within Child Custom fn - Before Critical area" = "Custom > within Child Custom fn - Before Critical area" [src\core\botmodules.rs:1366] "Core > After Execution Body is completed" = "Core > After Execution Body is completed" [src\core\botmodules.rs:1212] "SUCCESS! Routinecompleted" = "SUCCESS! Routinecompleted" ```
modulatingforce added 2 commits 2024-03-31 20:14:58 -04:00
modulatingforce reviewed 2024-03-31 20:24:16 -04:00
modulatingforce left a comment
Author
Owner
  • Need to address this before accepting the fix

We should use async custom fn definitions so we have the ability to await on Chat.say() calls

Don't believe this is easy to just revert back without possibility re-introducing the locks - but will need to check

- [ ] Need to address this before accepting the fix We should use `async` custom `fn` definitions so we have the ability to await on `Chat.say()` calls Don't believe this is easy to just revert back without possibility re-introducing the locks - but will need to check
@ -253,0 +282,4 @@
// [ ] !! ISSUE : With this fix though, the custom fn is no longer
// async. This is an issue because chat module is async
// - There should be no need to change chat , as async allows
// us to multitask with messages
Author
Owner

Issue with the Fix

If child Routine fn are defined as regular fn rather than async , Custom Routine fn would not be able to call Chat.say() or similar functionality

Here for example, in a Custom Routine fn innertester(params : Arc<RwLock<ExecBodyParams>> , we can't call chat.say() in this area

### Issue with the Fix If child Routine `fn` are defined as regular `fn` rather than `async` , Custom Routine `fn` would not be able to call `Chat.say()` or similar functionality Here for example, in a Custom Routine `fn innertester(params : Arc<RwLock<ExecBodyParams>>` , we can't call `chat.say()` in this area
Author
Owner

Honestly I'm favoring keeping the fox and maybe enhancing Chat so it has non-async calls for say().

Keep in mind in implementing, we cannot call async calls in a blocking context. This means if I have a non async code block, code calls (e.g. fn) cannot be awaited on. In other words in non Async fn, I cannot use client.send().await. There are ways around this though (later notes)

I don't foresee issues with multitasking here if we enhance Chat so users essentially enqueue "messages" (consisting of BotMsgType and ExecParams) then the bot has a separate tokio::task running that processes those messages (i.e., triggering the message using given twitch-irc call like client.say() )

In addition with in place, when custom module developers define a routine, they don't have to worry about locks as within the context of a custom built Routine (in theory) should be blocking

  • ofc we still need to be mindful of locks initiated and managed at core level

I'm thinking of using async-channel crate
https://docs.rs/async-channel/latest/async_channel/fn.bounded.html

Honestly I'm favoring keeping the fox and maybe enhancing `Chat` so it has non-async calls for `say()`. Keep in mind in implementing, we cannot call `async` calls in a blocking context. This means if I have a non `async` code block, code calls (e.g. `fn`) cannot be awaited on. In other words in non Async fn, I cannot use `client.send().await`. There are ways around this though (later notes) I don't foresee issues with multitasking here if we enhance `Chat` so users essentially enqueue "messages" (consisting of `BotMsgType` and `ExecParams`) then the bot has a separate `tokio::task` running that processes those messages (i.e., triggering the message using given `twitch-irc` call like `client.say()` ) In addition with in place, when custom module developers define a routine, they don't have to worry about locks as within the context of a custom built `Routine` (in theory) should be blocking - ofc we still need to be mindful of locks initiated and managed at `core` level --- I'm thinking of using `async-channel` crate https://docs.rs/async-channel/latest/async_channel/fn.bounded.html
Author
Owner

Hmm 🤔 I still need to review Tori's branch but wanted to mention the latest solution I started going for might not be the best path. I'm noticing some problems with what I've put in place so far (below), so I may separate my fixes out in separate branch

  • extremely high memory utilization. When testing, I've seen the bot take 11gb of memory 😰
    • I think this may be attributed to either added instrumentality and/or some of the restructuring
  • async multitasking is not occurring
    • This makes sense as I started adding blocking calls. But I noticed this is still occurring when I don't use blocking calls as well (I may have to recheck if the scenario I'm seeing now is one that existed before; if this existed before, I'll create a separate issue)

__

With above said, my solution's enhancements to chat are welcomed as this could be a workaround for locking issues if we can't identify how to address a lock, but willing to go down a non-async workflow

Probably will use this branch as the main issue branch, but continue working on my solution in a separate branch

Hmm 🤔 I still need to review Tori's branch but wanted to mention the latest solution I started going for might not be the best path. I'm noticing some problems with what I've put in place so far (below), so I may separate my fixes out in separate branch - extremely high memory utilization. When testing, I've seen the bot take 11gb of memory 😰 - I think this may be attributed to either added instrumentality and/or some of the restructuring - async multitasking is not occurring - This makes sense as I started adding blocking calls. But I noticed this is still occurring when I don't use blocking calls as well (I may have to recheck if the scenario I'm seeing now is one that existed before; if this existed before, I'll create a separate issue) __ With above said, my solution's enhancements to `chat` are welcomed as this could be a workaround for locking issues if we can't identify how to address a lock, but willing to go down a non-async workflow Probably will use this branch as the main issue branch, but continue working on my solution in a separate branch
modulatingforce added the
Status
Blocked
label 2024-04-05 12:21:31 -04:00
Author
Owner
  • Will need to review the two solutions before merging either or both

Just to note, I think making Routines define custom functions differently than other BotActions may get confusing for Custom Developers - so better to make them async across the board. In addition, we're giving Custom Developers async features ; if required under async, Custom Developers may be able to replicate blocking behavior if required

- [ ] Will need to review the two solutions before merging either or both Just to note, I think making Routines define custom functions differently than other BotActions may get confusing for Custom Developers - so better to make them `async` across the board. In addition, we're giving Custom Developers `async` features ; if required under `async`, Custom Developers may be able to replicate blocking behavior if required
This pull request is marked as a work in progress.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue-routine-lock:issue-routine-lock
git checkout issue-routine-lock

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout routines-functionality
git merge --no-ff issue-routine-lock
git checkout issue-routine-lock
git rebase routines-functionality
git checkout routines-functionality
git merge --ff-only issue-routine-lock
git checkout issue-routine-lock
git rebase routines-functionality
git checkout routines-functionality
git merge --no-ff issue-routine-lock
git checkout routines-functionality
git merge --squash issue-routine-lock
git checkout routines-functionality
git merge --ff-only issue-routine-lock
git checkout routines-functionality
git merge issue-routine-lock
git push origin routines-functionality
Sign in to join this conversation.
No reviewers
No milestone
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#40 WIP: Basic Routine Functionality}
modulatingforce/forcebot_rs
Reference: modulatingforce/forcebot_rs#51
No description provided.