sigmaSd / IRust

Cross Platform Rust Repl
https://crates.io/crates/irust
MIT License
622 stars 34 forks source link

Cannot use two `irust_repl`s at the same time. #114

Closed baiguoname closed 1 year ago

baiguoname commented 1 year ago

I have two object irust_repl: irust_repl1 and irust_repl2. When I call irust_repl1.eval(some_code) and irust_repl2.eval(some_code) at the same time, the latter has a link.exe error. I thought it maybe because that no matter how many irust_repl I have, they all is based on the same temp file, and so manupulating the same main.rs at the same time. So, if I need run the irust_repls at the same time, what should I do?

sigmaSd commented 1 year ago

Yes your guess is right it uses a hardcoded cargo project path, I can look later at making this configurable

sigmaSd commented 1 year ago

I can't reproduce the issue on linux , theoretically it shouldn't error , because the state is kept in memory and each time eval runs it will copy that state from memory to disk

The only problem I can see is that if the code runs in multiple threads at the same time, there is a risk of things getting messed up (like one repl accessing another repl values)

I think I should fix this issue by giving each repl its unique project, but I'm not 100 % sure it will fix your issue, if you have a minimal code that code can reproduce the issue that would be great

This is what I'm using to test the multi thread issue for example

#[test]
fn two_repls_at_the_same_time() {
    let mut repl1 = Repl::default();
    let mut repl2 = Repl::default();
    repl1.insert("let a = 4;");
    repl2.insert("let a = 5;");

    let a1_thread =
        std::thread::spawn(move || repl1.eval("a").unwrap().output.parse::<u8>().unwrap());
    let a2_thread =
        std::thread::spawn(move || repl2.eval("a").unwrap().output.parse::<u8>().unwrap());

    assert_eq!(a1_thread.join().unwrap() + a2_thread.join().unwrap(), 9) // this might fail it might equal to 10
}
baiguoname commented 1 year ago

I just test the code you provided above in Windows, it also works well. The original error I produced is run on web page, it may be too verbose to paste here. But I reproduce the same error in irust repl terminal, like this: 图片 Firstly, I open two terminals, I input the following codes into both of them: { use std::{thread::sleep, time::Duration}; let a = 1; sleep(Duration::from_secs(10)); a } And then , I run these two terminals at the same time. As can be seen in the picture, the second terminal raise the error.

sigmaSd commented 1 year ago

Can you test this pr https://github.com/sigmaSd/IRust/pull/115

baiguoname commented 1 year ago

I just test the new irust_repl, it works well. And what is amazing is that:

  1. A new irust_repl take short time to run for the first time;
  2. The directory temp/repls doesn't blow up as I create more and more irust_repl object.

I'm using a lot of IJulia, which is a repl for Julia. It is amazing but with a imperfection, by which the users suffer a lot: it takes too long for the first running. That problem also lies in evcxr , in a more severe way. I thought irust may solved this problem. Being grateful for your work, I thought in my personal view if irust have a full support for jupyter kernel, it will be a blazing edge for rust, and will contribute a lot to make rust more popular.

sigmaSd commented 1 year ago

Nice, glad it worked, the repl structs currently doesn't clean the created paths , though I exposed already a function to do that and irust uses it, I think I'll just go ahead and make it automatic by implementing it on drop (so it works as you described)

I'll try to make some time and evaluate jupyter again, did you try https://github.com/sigmaSd/IRust/tree/master/crates/irust_repl#jupyter-kernel btw ? does it do what you expect ? Its just a bare bone implementation, but it should do the basics. I think jupyter support only make sense after fixing https://github.com/sigmaSd/IRust/issues/104 , since after that adding jupyter should be straightforward

baiguoname commented 1 year ago

Yes, I tried https://github.com/sigmaSd/IRust/tree/master/crates/irust_repl#jupyter-kernel several times before on Windows, it didn't work. When I open jupyter and click the run button, it run down immediately and no output shows, the terminal also says the kernel is terminating. I guess there may be errors on the output part.

sigmaSd commented 1 year ago

I tried the kernel in linux and it seems to work, did you follow the steps ? can you see the kernel with jupyter kernelspec list, also is the re executable in your path, can you test it to make sure (re 1+2 for example)

I'll have to find a windows machine to test otherwise

baiguoname commented 1 year ago

I just tried again, re is executable in my path, and I run .\re 1+2 in the terminal can work well and irustkernel showed in the jupyter kernelspec list. But when I run the jupyter cell, the terminal has the same error. The following is the error: Traceback (most recent call last): File "C:\ProgramData\Anaconda3\lib\site-packages\ipykernel\kernelbase.py", line 353, in dispatch_shell await result File "C:\ProgramData\Anaconda3\lib\site-packages\ipykernel\kernelbase.py", line 643, in execute_request reply_content = self.do_execute( File "D:\Rust\IRust-master\crates\irust_repl\irustkernel\irust.py", line 29, in do_execute output = self.eval(self.repl + code, self.get_deps()) File "D:\Rust\IRust-master\crates\irust_repl\irustkernel\irust.py", line 48, in eval output = subprocess.run(["re", deps, code], stdout=subprocess.PIPE) File "C:\ProgramData\Anaconda3\lib\subprocess.py", line 505, in run with Popen(*popenargs, **kwargs) as process: File "C:\ProgramData\Anaconda3\lib\subprocess.py", line 951, in __init__ self._execute_child(args, executable, preexec_fn, close_fds, File "C:\ProgramData\Anaconda3\lib\subprocess.py", line 1420, in _execute_child hp, ht, pid, tid = _winapi.CreateProcess(executable, args), FileNotFoundError

sigmaSd commented 1 year ago

That error mean that your path is not set up correctly, you can try changing 're' here https://github.com/sigmaSd/IRust/blob/master/crates/irust_repl/irustkernel/irust.py#L48 to the absolute path of the binary (C:://...path..here../re)

baiguoname commented 1 year ago

I just tried that, and then there is a new type of error: thread 'main' panicked at 'No code provided', crates\irust_repl\examples\re\main.rs:7:14 note: run withRUST_BACKTRACE=1environment variable to display a backtrace thread 'main' panicked at 'No code provided', crates\irust_repl\examples\re\main.rs:7:14 note: run withRUST_BACKTRACE=1environment variable to display a backtrace

sigmaSd commented 1 year ago

Can you add a print(code) before this line https://github.com/sigmaSd/IRust/blob/master/crates/irust_repl/irustkernel/irust.py#L48

sigmaSd commented 1 year ago

Also can we continue this conversation here instead https://github.com/sigmaSd/IRust/issues/108

baiguoname commented 1 year ago

I just tried it, it has the same error: image

sigmaSd commented 1 year ago

Maybe you need to run jupyter trust notebook-name.ipynb https://stackoverflow.com/questions/44943646/jupyter-notebook-not-trusted

sigmaSd commented 1 year ago

also if that doesn't work I'll need a print here https://github.com/sigmaSd/IRust/blob/master/crates/irust_repl/irustkernel/irust.py#L20 , if there is no output that means jupyter is not sending any code to the kernel

baiguoname commented 1 year ago

First, I tried trust irust.ipynb, then run the cell, it has the same error; Then, I add the print code to where as your point, it still has the same error: image

sigmaSd commented 1 year ago

I mean you should add print and see the console, its for debugging, I want to see if the code gets printed

baiguoname commented 1 year ago

The console also didn't print the code. And I found this warning in the irust.py: image

sigmaSd commented 1 year ago

The warning is probably harmless since it does execute, it's weird that do_execute doesn't pass code, if you want to debug more you can try the echo kernel from here https://jupyter-protocol.readthedocs.io/en/latest/wrapperkernels.html

sigmaSd commented 1 year ago

Btw for the path issue I remembered in windows you might need cmd prefix so instead of run(["re") you can try run(["cmd","/c", "re"

baiguoname commented 1 year ago

I added the cmd prefix, it has the same error: image

sigmaSd commented 1 year ago

Ok so the cmd prefix does fix the first issue

sigmaSd commented 1 year ago

you seem to have reamoved Code and deps variable, It should be run(cmd,/c,re,deps,code)

baiguoname commented 1 year ago

Yes, after I add deps code, it can run fine now. But no output be printed: image

sigmaSd commented 1 year ago

Yes thats probably normal, jupyer is not printing it , for debugging we should print to stderr instead so it shows up in the console

Can you try btw run("cmd","/c","re",deps,code) instead of the absolute path of re , does that still work ?

IF you have some feedback /suggestions / new issues on this kernel feel free to add it here https://github.com/sigmaSd/IRust/issues/108

baiguoname commented 1 year ago

Now everything works fine, and I put the comments to #108 .