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

Document thread-safety of librpm #1483

Closed
DemiMarie opened this issue Jan 11, 2021 · 9 comments · Fixed by #1516
Closed

Document thread-safety of librpm #1483

DemiMarie opened this issue Jan 11, 2021 · 9 comments · Fixed by #1516
Assignees

Comments

@DemiMarie
Copy link
Contributor

I have not been able to determine if librpm is thread-safe. From looking at the code, it appears not to be, for several reasons:

  • librpm changes global state, such as the process umask.
  • Lua scripts can change the environment, which can race with access to the environment from other threads. They can also change the working directory, which would be a problem in multithreaded use.
  • Lua code can call fork() and allocate memory before calling exec(). In a multithreaded process, this is undefined behavior according to POSIX.
  • librpm calls plugin hooks after forking. There is no guarantee that these hooks only use async-signal-safe interfaces.

This is merely a request for documentation of the status quo. The best way to use librpm in a multithreaded process is most likely to spawn the rpm binary. Parts of librpm, such as signature verification, are probably thread-safe, but running transactions probably isn’t.

@pmatilai pmatilai self-assigned this Jan 15, 2021
pmatilai added a commit to pmatilai/rpm that referenced this issue Jan 25, 2021
pmatilai added a commit that referenced this issue Jan 26, 2021
@DemiMarie
Copy link
Contributor Author

Are there problems due to concurrent calls to chdir() or calls to setenv by Lua scripts? Also, Lua scripts can call fork(), allocate memory, and then exec(), which POSIX only allows in a single-threaded process.

To me, it seems safer to only invoke the RPM transaction APIs when only one thread is running. @pmatilai thoughts?

@pmatilai
Copy link
Member

Well yes, most people will find the going along a running transaction a bit too much when not just the current directory but also the root can change and whatnot.

@DemiMarie
Copy link
Contributor Author

The reason I am asking is that I am working on (not yet published) Rust bindings to a small part of the RPM API. Rust requires that safe code not be able to invoke undefined behavior. Is it safe to fork() and then perform an RPM transaction in the child process, even if the parent was multithreaded?

@cgwalters
Copy link
Contributor

cgwalters commented Jan 27, 2021

FWIW in rpm-ostree we reimplement most of the RPM install path for multiple reasons (among them we "snapshot" multiple RPM versions into ostree commits as part of implementing transactional updates, we want to sandbox scripts etc.) but another big reason is that we simply cannot have librpm call chroot() - that has process-global effects and rpm-ostree is multi-threaded (like nearly all nontrivial software).

(Also on this topic btw rpm-ostree is oxidizing fast now, see e.g. coreos/rpm-ostree#2502 which I just submitted - so if there's RPM+Rust topics it might come up there too)

@DemiMarie
Copy link
Contributor Author

@cgwalters Nice! I’m working on Rust bindings to a small part of the RPM API.

@pmatilai
Copy link
Member

Is it safe to fork() and then perform an RPM transaction in the child process, even if the parent was multithreaded?

It depends on so many factors I can't possibly answer such a question.

@DemiMarie
Copy link
Contributor Author

Is it safe to fork() and then perform an RPM transaction in the child process, even if the parent was multithreaded?

It depends on so many factors I can't possibly answer such a question.

Does librpm use pthread_atfork to protect its locks, and do the libraries librpm depends on do so?

@DemiMarie
Copy link
Contributor Author

RPM depends on SQLite which cannot safely used in a child process after fork. So the answer is “no”, meaning that the only reasonable approach is to perform an RPM transaction using a separate binary.

@Conan-Kudo
Copy link
Member

Conan-Kudo commented Feb 3, 2021

Nice! I’m working on Rust bindings to a small part of the RPM API.

@DemiMarie Note that there are already RPM Rust bindings that you can contribute to: https://github.com/rpm-software-management/librpm.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Reference manual
Awaiting triage
Development

Successfully merging a pull request may close this issue.

4 participants