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

#7 と #60 の実装について意見を伺いたい #65

Closed
wants to merge 6 commits into from

Conversation

hayatroid
Copy link
Collaborator

関連Issue

概要

意見を伺いたいです.

#7#60 を実現するため,以下のように text_file.rs の実装をしました.

ファイルの依存関係を,オリジナルファイルを根,シンボリックリンクのリンク先を親,リンク元を子とした根付き木として考えたときに,

  • 葉から drop する構造を実現するため,
    • 子に Arc<親> を持たせた.
    • これを実現するため,TextFileInner と,これを Arc でラップした TextFile の 2 つの構造体を用意した.
    • tests.rs を書いて正しく動作していそうなことを確認した.
  • 根付き木全体で 1 つの RwLock<FileAccess> を共有するため,
    • オリジナルファイルに Arc<RwLock<FileAccess>> を作成して持たせ,子孫にこれの clone を持たせた.

しかし,以下の状況に直面しました.

  • create_symlink_todrop とでどちらの構造体に実装したいかが異なる.
    • create_symlink_toself.clone() を子に持たせるために Arc でラップされた TextFile 側に実装したい.
    • drop は 1 ファイルに 1 回だけ呼び出されるように Arc でラップされていない TextFileInner 側に実装したい.

トレイトを書き換えれば解決することではありますが,

  • これよりもっと素直な実装はないか
  • そもそもこれが正しく動作しそうか(Arc 周りの理解が浅くあまり自身がないです)

など意見を伺いたいです.よろしくお願いします.

@comavius comavius added the enhancement New feature or request label Oct 13, 2024
@comavius
Copy link
Collaborator

PRありがとう、説明をサボったままお願いしてしまって本当に申し訳ないです。
下のテストを実行するとoutputが出力されるけどexpectedのようになってほしいです。

#[test]
fn drop_not_leaf_node() -> anyhow::Result<()> {
    let a = TextFile::new(PathBuf::from("a.txt"), "hello!".to_string())?;

    let b = a.create_symlink_to(PathBuf::from("b.txt"))?;

    let c = b.create_symlink_to(PathBuf::from("c.txt"))?;

    std::mem::drop(b);
    
    println!("b is dropped.");

    Ok(())
}
/*
output: 
  b1 is dropped.
  "c.txt"
  "b.txt"
  "a.txt"
expected:
  "b.txt"
  b1 is dropped.
  "c.txt"
  "a.txt"
*/

これは、今は木構造が

entity - linkA - linkB - linkC

になっているからcがdropするまでbがdropしないからで、辺を

entity - linkA
      |- linkB
      |- linkC

のように貼って、a,b,cそれぞれがArc<Lock>を持つようにすれば、木構造を壊さずに適時ファイルが消えるようになるしdropも各ファイルの実体に対して必ず1回走るようになる気がします。
なので、

  • Entity型とLink型を作ってLinkがArc<Lock>を持つと素直そう?(このあたりは自身がないです)
  • linkがlinkの参照を持つと期待された動きをしなさそう

になると思います。

@hayatroid
Copy link
Collaborator Author

レビューありがとう,根に直接リンクを繋ぐのが素直そうですね,その方針で実装してみたいと思います 👍

@hayatroid hayatroid closed this Oct 14, 2024
@hayatroid hayatroid reopened this Oct 14, 2024
@hayatroid
Copy link
Collaborator Author

@comavius
重ねての質問ですみません.

Link が Arc<Entity 全体> を持つと Link から drop されることが保証されていそうですが,Link が Arc<Entity の中にある Lock> を持つと Link から drop されることが保証されないような気がしています(そもそも Lock は Entity のフィールドにあるという認識で合っていますか).

もし Link が Arc<Entity 全体> を持たないといけないとなると,初めに述べたような状況になってしまいそうで難しいです.何か良い方法がないか(そもそも認識が合っているか)を伺いたいです.

@comavius
Copy link
Collaborator

struct Link {
  entity: Arc<RwLock<Entity>>,
  //...
}

であれば、
Linkがdrop
-> Arc<RwLock>がdrop
-> (もしArcの参照カウントが0になったら)RwLockがdrop
-> Entityがdrop
となりませんか?

@hayatroid
Copy link
Collaborator Author

hayatroid commented Oct 15, 2024

それだと動作すると思います.ただ,どこかで RwLock<Entity> に対して Arc::new() をして,リンクを貼るときにその clone を持ってきて Link に持たせることになると思うのですが,Arc::new() したやつをどこに置いとけばよいのかがわからないです.

(たとえば初めに示した実装だと OuterArc<Inner> を持つことでこれを実現しているのですが,あまり上手くない気がしています.)

@hayatroid
Copy link
Collaborator Author

@comavius
ざっくりと下記のような実装を考えているのですが,Entity に対する create_symlink_to() を実装するところで詰まっています(↑ で述べた理由によってです).解決策が浮かばず,助けていただきたいです 🙇

struct Entity {
    path: PathBuf,
}

impl File for Entity {
    type InitArgs = String;
    type Link = Link;
    fn new(path: PathBuf, args: Self::InitArgs) -> anyhow::Result<Self> {
        // ここでファイル作成する
        Ok(Entity { path })
    }
    fn create_symlink_to(&self, path: PathBuf) -> anyhow::Result<Self::Link> {
        Link::new(
            path,
            unimplemented!("ここで Arc<RwLock<Entity>> の clone を渡したいが,どこから持ってくればよいのかわからない."),
        )
    }
}

struct Link {
    path: PathBuf,
    entity: Arc<RwLock<Entity>>,
}

impl File for Link {
    type InitArgs = Arc<RwLock<Entity>>;
    type Link = Link;
    fn new(path: PathBuf, args: Self::InitArgs) -> anyhow::Result<Self> {
        // ここでシンボリックリンク作成する
        Ok(Link {
            path,
            entity: args.clone(),
        })
    }
    fn create_symlink_to(&self, path: PathBuf) -> anyhow::Result<Self::Link> {
        Link::new(path, self.entity.clone())
    }
}

@comavius
Copy link
Collaborator

@hayatroid
この場合、EntityとLinkが同じトレイトを実装する必要はなく、Entity側にlinkを発行する機能をもたせる必要はないと思います。

@hayatroid
Copy link
Collaborator Author

@comavius
text_file.rs のレビューをお願いします.Entity に対する create_symlink_tounimplemented! にしておきました(new, drop は欲しいかなと思ったので File トレイトは実装したままにしておきました).使い方は tests.rs にあるように下記のようなものを想定しています.

#[test]
fn drop_not_leaf_node() -> anyhow::Result<()> {
    let a = Arc::new(RwLock::new(TextFileEntity::new(
        PathBuf::from("a.txt"),
        "hello".to_string(),
    )?));

    let b = TextFileLink::new(PathBuf::from("b.txt"), a.clone())?;

    let c = b.create_symlink_to(PathBuf::from("c.txt"))?;

    drop(b);

    println!("b is dropped.");

    Ok(())
}

これの出力は

Link "b.txt" dropped.
b is dropped.
Link "c.txt" dropped.
Entity "a.txt" dropped.

となり,要件を満たしていると思います.

よさそうだったらデバッグ出力を消して,Directory に対しても同じ実装をして,FileFactory の実装もして,もう 1 つの方の PR に投げようと思っています.よろしくお願いします.

@comavius
Copy link
Collaborator

comavius commented Oct 18, 2024

@hayatroid もう一点、Fileトレイトについて、Entityはsymlink発行をしないので、FileEntityトレイトとFileLinkトレイトに分けるか、共通部分があるならtrait FileLink : File {...trait FileEntity: File {...というようにするかをお願いしたいです

@hayatroid hayatroid closed this Oct 25, 2024
@hayatroid hayatroid deleted the draft/#7-add-custom-file branch October 25, 2024 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants