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

Fix crash when a writer is in the global namespace #539

Merged
merged 6 commits into from
Jan 16, 2024

Conversation

jmcarcell
Copy link
Member

BEGINRELEASENOTES

  • Fix crash when a writer is in the global namespace by adding a class that will manage all the writers and finish them before exiting

ENDRELEASENOTES

The following code crashes (either Writer or RNTupleWriter):

import podio
writer = podio.root_io.RNTupleWriter('rntuple.root')

The workaround is to add a del writer at the end. This happens because the writers do something when they are destructed but podio and/or ROOT may get destructed before. Error message:

Error in <TKey::Create>: Cannot allocate 658 bytes for ID =  Title =
SysError in <TFile::Seek>: cannot seek to position 0 in file rntuple.root, retpos=-1 Bad file descriptor
Fatal: !rv violated at line 1152 of `/root/tree/ntuple/v7/src/RMiniFile.cxx'
aborting

Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

This does also affect all the readers, right? Could we the _all_writers.add into the BaseReaderMixin constructor?

@jmcarcell
Copy link
Member Author

Hm nope, at least they don't crash. I've moved the management to base_writer.py, in lines of code it's the same since every writer has now to do super().__init__()

Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Not really sure about the AllWriters name, but I can't think of anything better. Since this is internal in any case we should be able to change this later if we come up with something better.

@tmadlener tmadlener merged commit 276cdaa into AIDASoft:master Jan 16, 2024
18 checks passed
@jmcarcell
Copy link
Member Author

WriterManager, WriterDeleter or WriterFinisher or even WriterDestroyer for maximum effect

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.

2 participants