Skip to content

[io] Prevent losing data due to SIGTERM (under Unix) #18403

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/base/inc/TROOT.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ friend TROOT *ROOT::Internal::GetROOT2();
Int_t Timer() const { return fTimer; }

//---- static functions
static void CleanUpROOTAtExit();
static Int_t DecreaseDirLevel();
static Int_t GetDirLevel();
static const char *GetMacroPath();
Expand All @@ -348,6 +349,7 @@ friend TROOT *ROOT::Internal::GetROOT2();
static Int_t ConvertVersionCode2Int(Int_t code);
static Int_t ConvertVersionInt2Code(Int_t v);
static Int_t RootVersionCode();
static void WriteCloseAllFiles();
static const std::vector<std::string> &AddExtraInterpreterArgs(const std::vector<std::string> &args);
static const char**&GetExtraInterpreterArgs();

Expand Down
29 changes: 28 additions & 1 deletion core/base/src/TROOT.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,37 @@ static Int_t ITIMQQ(const char *time)
return 100*hh + mm;
}

////////////////////////////////////////////////////////////////////////////////
/// Write and Close all open writable TFiles, useful to be called when SIGTERM is caught.

void TROOT::WriteCloseAllFiles()
{
if (gROOT) {
R__LOCKGUARD(gROOTMutex);

if (gROOT->GetListOfFiles()) {
TIter next(gROOT->GetListOfFiles());
while (TObject *obj = next()) {
if (obj && obj->InheritsFrom(TClass::GetClass("TFile", kFALSE, kTRUE))) {
TMethodCall callIsWritable(obj->IsA(), "IsWritable", "");
Longptr_t retLong = 0;
callIsWritable.Execute((void *)(obj), retLong);
if (retLong == 1) {
TMethodCall callWrite(obj->IsA(), "Write", "");
callWrite.Execute((void *)(obj));
TMethodCall callClose(obj->IsA(), "Close", "");
callClose.Execute((void *)(obj));
}
}
}
}
}
}

////////////////////////////////////////////////////////////////////////////////
/// Clean up at program termination before global objects go out of scope.

static void CleanUpROOTAtExit()
void TROOT::CleanUpROOTAtExit()
{
if (gROOT) {
R__LOCKGUARD(gROOTMutex);
Expand Down
7 changes: 7 additions & 0 deletions core/textinput/src/textinput/TerminalConfigUnix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <stdio.h>
#include <cstring>

#include <TROOT.h>

using namespace textinput;
using std::memcpy;
using std::signal;
Expand Down Expand Up @@ -104,6 +106,11 @@ TerminalConfigUnix::HandleSignal(int signum) {
}
}

// gentle save and close if SIGTERM
if (signum == SIGTERM) {
TROOT::WriteCloseAllFiles();
TROOT::CleanUpROOTAtExit();
}
// No previous handler found, re-raise to get default handling:
signal(signum, SIG_DFL); // unregister ourselves
raise(signum); // terminate through default handler
Expand Down
2 changes: 1 addition & 1 deletion io/io/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# For the list of contributors see $ROOTSYS/README/CREDITS.

ROOT_ADD_GTEST(RRawFile RRawFile.cxx LIBRARIES RIO)
ROOT_ADD_GTEST(TFile TFileTests.cxx LIBRARIES RIO)
ROOT_ADD_GTEST(TFile TFileTests.cxx LIBRARIES RIO Hist)
ROOT_ADD_GTEST(TBufferFile TBufferFileTests.cxx LIBRARIES RIO)
ROOT_ADD_GTEST(TBufferMerger TBufferMerger.cxx LIBRARIES RIO Imt Tree)
ROOT_ADD_GTEST(TBufferJSON TBufferJSONTests.cxx LIBRARIES RIO)
Expand Down
24 changes: 24 additions & 0 deletions io/io/test/TFileTests.cxx
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
#include <memory>
#include <vector>
#include <string>
#include <csignal>

#include "gtest/gtest.h"

#include "TFile.h"
#include "TH1I.h"
#include "TMemFile.h"
#include "TDirectory.h"
#include "TKey.h"
Expand Down Expand Up @@ -201,3 +203,25 @@ TEST(TFile, MakeSubDirectory)
EXPECT_EQ(std::string(gDirectory->GetPath()), "dirTest17824.root:/a/b/c");
EXPECT_EQ(std::string(gDirectory->GetName()), "c");
}

// https://github.com/root-project/root/issues/13300
TEST(TFile, Sigterm)
{
auto filename = "out13300.root";
{
TFile file(filename, "RECREATE");
file.mkdir("subdir")->cd();
TH1I hist("h", "h", 10, 0., 1.);
hist.Fill(0.4);
// Since the real behavior is save+close+crash which would make the test fail,
// rather than calling directly std::raise(SIGTERM),
// we emulate the response to SIGTERM in TerminalConfigUnix before crash:
TROOT::WriteCloseAllFiles();
TROOT::CleanUpROOTAtExit();
}
{
TFile file(filename, "READ");
ASSERT_EQ(file.Get<TH1I>("subdir/h")->GetBinContent(5), 1);
}
gSystem->Unlink(filename);
}
Loading