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

Make MemoryDatabase Send + Sync #448

Conversation

artfuldev
Copy link
Contributor

@artfuldev artfuldev commented Oct 13, 2021

Description

We make the MemoryDatabase Send + Sync. This is required for language bindings generation using uniffi-rs and is a very small change.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@artfuldev artfuldev force-pushed the use-send-and-sync-on-memory-database branch from c0af613 to b1beaf6 Compare October 13, 2021 23:32
@artfuldev artfuldev marked this pull request as ready for review October 13, 2021 23:33
@LLFourn
Copy link
Contributor

LLFourn commented Oct 13, 2021

This LGTM. I think the idea here is to make sure that if you Arc<Mutex<>> the in memory database it will be send + sync as expected. Am I right?

No idea why these resolver test failures are reappearing in our CI.

@artfuldev
Copy link
Contributor Author

I'm not sure I understand the Arc<Mutex<>> part. The only way to make it automatically Send + Sync is to ensure the inner-most contained type Box<dyn Any> is also Send + Sync - that was the only violation, so I added the trait constraint there.

@artfuldev
Copy link
Contributor Author

No idea why these resolver test failures are reappearing in our CI.

Is this something to be fixed by me?

@LLFourn
Copy link
Contributor

LLFourn commented Oct 13, 2021

Is this something to be fixed by me?

Nah we should fix this.

@afilini
Copy link
Member

afilini commented Oct 15, 2021

The issue should be fixed now, you can try rebasing on the latest master

@artfuldev artfuldev force-pushed the use-send-and-sync-on-memory-database branch from b1beaf6 to 7d5891b Compare October 15, 2021 16:05
@artfuldev artfuldev force-pushed the use-send-and-sync-on-memory-database branch from 7d5891b to 59f795f Compare October 15, 2021 16:06
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 59f795f.

@notmandatory notmandatory merged commit 59f795f into bitcoindevkit:master Oct 16, 2021
@artfuldev artfuldev deleted the use-send-and-sync-on-memory-database branch October 16, 2021 23:19
@artfuldev artfuldev restored the use-send-and-sync-on-memory-database branch October 28, 2021 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants