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

Remove/fix prettytable-rs dependency #195

Closed
cschwan opened this issue Dec 8, 2022 · 12 comments
Closed

Remove/fix prettytable-rs dependency #195

cschwan opened this issue Dec 8, 2022 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@cschwan
Copy link
Contributor

cschwan commented Dec 8, 2022

Recently, ubuntu-latest was switched from ubuntu-20.04 to ubuntu-22.04, which seems to break pineappl. It segfaults, see https://github.com/NNPDF/pineappl/actions/runs/3646167214/jobs/6156975988. My bet is that it's not caused by pineappl itself but rather by a dependency.

@cschwan cschwan self-assigned this Dec 8, 2022
@cschwan cschwan changed the title Figure out why pineappl segfaults in CI with ubuntu-22.04 Figure out why pineappl segfaults in CI Dec 8, 2022
@alecandido
Copy link
Member

alecandido commented Dec 8, 2022

I can tell you that I could compile PineAPPL on my local Ubuntu 22.04, I did it many times.
And run the CLI as well, of course.

@cschwan
Copy link
Contributor Author

cschwan commented Dec 8, 2022

Yes, it's not quite that easy. What I noticed is that it started breaking when the switch happened, but it's still there with ubuntu-20.04.

It seems that pineappl [subcommand] --help is fine, while operations involving convolutions segfault: https://github.com/NNPDF/pineappl/actions/runs/3646494375/jobs/6157637600.

@alecandido
Copy link
Member

alecandido commented Dec 8, 2022

I had a look to the logs, but it is highly non-trivial.
Especially, I do not see what they all have in common: consider that also commands like pineappl obl are failing, and no convolution should be involved there.

Maybe the more precise statements is about grid loading: --help does not load any grid.

@cschwan
Copy link
Contributor Author

cschwan commented Dec 8, 2022

True, that is even weirder.

@cschwan
Copy link
Contributor Author

cschwan commented Dec 8, 2022

I can reproduce a segfault with Rust nightly, which segfaults in prettytable::Table::printstd - that makes sense with the observations above, they all print a table using prettytable-rs.

@alecandido
Copy link
Member

I agree it matches the observation, but is the CI running with Nightly?

@cschwan
Copy link
Contributor Author

cschwan commented Dec 8, 2022

Yes, it was. It works with stable, see here: https://github.com/NNPDF/pineappl/actions/runs/3646808485/jobs/6158307245. However, I think the problem is prettytable-rs, and Rust nightly is probably just more strict than stable (in some way I don't know). Since you can't produce segfaults in Rust unless you use unsafe, my bet is on

https://github.com/phsym/prettytable-rs/blob/e159600d999f55f49fa9141e1a014d776a67c836/src/lib.rs#L513-L518

which is the only piece marked 'unsafe'. I can confirm that this piece of code is called.

@alecandido
Copy link
Member

alecandido commented Dec 8, 2022

Interestingly enough, the guy wrote:

// All this is a bit hacky. Let's try to find something else

on May 29, 2017, and it's still there.

It looks a good candidate to break with new versions. Essentially, it is recasting an immutable reference to a table to a mutable one, to perform a single operation. But that's an entry point for plenty of issues.

(and it is the only unsafe in the whole library in master, even though we should check the tag for the version used...)

@alecandido
Copy link
Member

Ok, it seems like there is already an issue open mentioning it phsym/prettytable-rs#145.
Is this matching the usage in pineappl_cli? @cschwan

@cschwan
Copy link
Contributor Author

cschwan commented Dec 8, 2022

Yes, I'm pretty sure that's the cause.

@cschwan cschwan changed the title Figure out why pineappl segfaults in CI Remove/fix prettytable-rs dependency Dec 16, 2022
@cschwan cschwan added the bug Something isn't working label Dec 16, 2022
@pinkforest
Copy link

You can just bump prettytable-rs to 0.10 - I've pushed a release :)

@cschwan
Copy link
Contributor Author

cschwan commented Dec 27, 2022

@pinkforest I'm already on it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants