Seg Fault when assigning sensitive detectors from array

I have a simulation made from 80 distinct volumes all of which I want to make “sensitive”. To facilitate this I have a text file which contains all the information I need to make the required materials and volumes and I store the pointers to all of my volumes in arrays. I similarly create 80 distinct instances of my sensitive detector class and assign each of them to my volumes.

My code compiles without objection but I get a set fault at runtime:

*** Break *** segmentation violation
===========================================================

There was a crash.

This is the entire stack trace of all threads:

===========================================================

#0 0x00007f962d4d6457 in __GI___waitpid (pid=6056, stat_loc=stat_loc

entry=0x7ffc513c92e8, options=options

entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:30

#1 0x00007f962d441177 in do_system (line=<optimized out>) at ../sysdeps/posix/system.c:149
#2 0x00007f963089ade3 in TUnixSystem::StackTrace() () from /opt/root/root-6.22.02-build/lib/libCore.so
#3 0x00007f963089d8d5 in TUnixSystem::DispatchSignals(ESignals) () from /opt/root/root-6.22.02-build/lib/libCore.so
#4 <signal handler called>
#5 0x00007f962eb22560 in G4LogicalVolume::GetSensitiveDetector() const () from /home/students/GEANT4/g4install/lib/libG4geometry.so
#6 0x00007f962fb6b402 in G4VUserDetectorConstruction::SetSensitiveDetector(G4LogicalVolume*, G4VSensitiveDetector*) () from /home/students/GEANT4/g4install/lib/libG4run.so
#7 0x000055af270e037b in ConstructJupiter::ConstructSDandField() ()
#8 0x00007f962fb3ccff in G4RunManager::InitializeGeometry() () from /home/students/GEANT4/g4install/lib/libG4run.so
#9 0x00007f962fb3cb71 in G4RunManager::Initialize() () from /home/students/GEANT4/g4install/lib/libG4run.so
#10 0x000055af270de22c in main ()

===========================================================

This error does not persist when I comment out the code which assigns the sensitive detectors to my logical volumes.

I create the arrays in my header file

G4SDManager* DetectorManager;
G4VSensitiveDetector *SDetectors[num_layers];

And the actual detectors + assignment in my source file

void ConstructJupiter::ConstructSDandField()
{
  DetectorManager = G4SDManager::GetSDMpointer();

  for(int i = 0; i < num_layers; i++)
  {
    SDetectors[i] = new JupiterSD("SensitiveAtmo");
    DetectorManager -> AddNewDetector(SDetectors[i]);
    layers_logical[i] -> SetSensitiveDetector(SDetectors[i]);
  }
}

(I have tried doing the assignment with the SetSensitiveDetector() method to no avail)
I have tried to create an individual instance of a sensitive detector and assign it to a single volume but without success which leads me to suspect that the problem lies in the logical volumes (though they seem to become physical volumes without complaint).

The code I use to create the volumes is here:

ifstream geometry_file;
geometry_file.open("Layers.txt");
int i = -1;
Rin = 0*km;

while (getline(geometry_file, line))
{
  i += 1;
  layers_solid[i] = new G4Sphere("Atmoshpere_Solid", Rin, Rin + stod(line)*km, 0*degree, 306*degree, 0*degree, 180*degree);
  layers_logical[i] = new G4LogicalVolume(layers_solid[i], materials[i], "Atmoshpere_Logical");
  layers_physical[i] = new G4PVPlacement(0, G4ThreeVector(0, 0, 0), layers_logical[i], "Atmoshpere_Physical", WorldLogical, false, CopyAtmo);
  Rin += stod(line)*km;
}
geometry_file.close();

If anything obvious in these code snippets stands out please let me know (I suspect that it has something to do with pointers/values but, I admit, I am not quite as proficient in C++ as I’d like to be). If nothing stands out I have also attached the source and header files for the relevant pieces.

ConstructJupiter.cc (6.3 KB) JupiterSD.cc (2.2 KB) Layers.txt (261 Bytes) ConstructJupiter.hh (2.8 KB) JupiterSD.hh (543 Bytes)

Don’t unconditionally create and register an SD. Query the SDManager first to see if it’s already been registered; if not, only then create one and add it to the SDManager.

Each SD instance needs to have a unique name, so that the SDManager can register it and find it, and can associate it with hit collections.

You should be able to do what you’re doing with one single SD instance, attached to all the different LVs. In your geometry loop, you are setting every one of your PVPlacements with copy number 0. Instead, use the loop index ‘i’ as the copy number of each layer. Then the SD can identify which layer contains the hit by querying the toucable for the copy number.

Thank you very much for your reply, I did not know that I was allowed to assign a single SD to multiple logical volumes, Im glad to know that this is possible and have implement that change. I also modified the creation of my PV’s so that each volume has a unique copy number. The new SD method looks like:

void ConstructJupiter::ConstructSDandField()
{
  DetectorManager = G4SDManager::GetSDMpointer();

  SDetector = new JupiterSD("SensitiveAtmo");
  DetectorManager -> AddNewDetector(SDetector);

  for(int i = 0; i < num_layers; i++)
  {
    layers_logical[i] -> SetSensitiveDetector(SDetector);
  }
}

This has, unfortunately, not resolved my problem. I have a slightly different stack trace:

===========================================================

There was a crash.

This is the entire stack trace of all threads:

===========================================================

#0 0x00007f00a93b0457 in __GI___waitpid (pid=21321, stat_loc=stat_loc

entry=0x7ffdda14d668, options=options

entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:30

#1 0x00007f00a931b177 in do_system (line=<optimized out>) at ../sysdeps/posix/system.c:149
#2 0x00007f00ac774de3 in TUnixSystem::StackTrace() () from /opt/root/root-6.22.02-build/lib/libCore.so
#3 0x00007f00ac7778d5 in TUnixSystem::DispatchSignals(ESignals) () from /opt/root/root-6.22.02-build/lib/libCore.so
#4 <signal handler called>
#5 0x00007f00aa9fc58c in G4LogicalVolume::SetSensitiveDetector(G4VSensitiveDetector*) () from /home/students/GEANT4/g4install/lib/libG4geometry.so
#6 0x0000556e6f50c473 in ConstructJupiter::ConstructSDandField() ()
#7 0x00007f00aba16cff in G4RunManager::InitializeGeometry() () from /home/students/GEANT4/g4install/lib/libG4run.so
#8 0x00007f00aba16b71 in G4RunManager::Initialize() () from /home/students/GEANT4/g4install/lib/libG4run.so
#9 0x0000556e6f50a2cc in main ()

===========================================================

as you can see, step 5 now points, as expected, to the “SetSensitiveDetector” method.

I have not attempted to implement your first recommendation since I have not found an example of this being done. Is the ConstructSDandField method called multiple times by GEANT? In the past I have never had this problem.

On a separate note: while testing I realized that my materials appear to not be in the material table. GEANT does not complain about this and, if this is an issue, I wonder why it would not cause an error until the assignment of sensitive detectors.

ConstructJupiter.cc (6.2 KB) ConstructJupiter.hh (2.3 KB)

You’re still creating the SD unconditionally. Instead the plain code, you want to have something like

G4String SDname = "SensitiveAtmo";
JupiterSD* SDetector = DetectorManager->FindSensitiveDetector(SDname, false);
if (!SDetector) {       // Need to create new SD for registration
  SDetector = new JupiterSD(SDname);
  DetectorManager->AddNewDetector(SDetector);
}

It looks like your “SDetector” variable is a data member of your ConstructJupiter class. You don’t want to do that…

Yes, it is! When you’re running multithreaded, it gets called by both the master thread and by each of the worker threads, so that there is a different, unique SD instantiated for each worker. That way, there are no mutexes or other conflicts to deal with between the SDs (which also means you shouldn’t have any static data in your SD class!).

Note that there’s only one detector construction instance, which is shared across threads. In your code above, the SDetector data member will be getting stomped on by the different threads. That shouldn’t cause a segfault directly, but it could be doing so indirectly, as one thread changes the value while another thread is in the middle of the layers loop.

If you keep the SD pointer local to the function, then each thread has its own and there’s no contention.

I am not running this multithreaded, even still I attempted your fix to no avail. The error is unchanged.

Even if you’re not running with more than one thread, was your Geant4 installed with G4MULTITHREADED enabled? That’s the default behaviour, so you would have had to explicitly turn it off. However, you’re correct that with only a single worker thread, you won’t get contention for the SDetector data member.

I notice that your traceback doesn’t have any details. Would it be difficult for you to build your application with debugging enabled? If you use CMake, just set the “release type” to Debug; if you’re using G4’s old-style GNUmakefile interface, you cat set the envvar G4DEBUG=1.

That would give you more information in the traceback, for example, function argument values, pointers, and so on, to see whether there’s a null pointer being referenced. Plus, with debugging enabled, you can run in GDB (or LLDB) and step through the function to see exactly what’s happening.

using -DCMAKE_BUILD_TYPE=Debug has not changed my traceback. Is this what you meant by setting the “release type” to debug.

I admit, with some embarrassment, that Im not sure which type of make I am using. I am using GEANT 4.10.06 and I call cmake to generate a makefile which I then “compile” with regular GNUmake.

I have solved the issue. My project is built to a build directory but I keep the source files (including the text files which contain the necessary info) in a source directory. Since I execute the program from the build directory my program failed to locate and open the text files it needed and therefore never made the materials or the geometries I needed. The segmentation fault occurred because I had no actual logical volumes constructed. I have since corrected this mistake.

Thank you so much @mkelsey you were a huge help and I’ve learned a lot more about GEANT from you.

Okay, you’re using CMake :slight_smile: CMake works by generating it’s own Makefile (with a deep tree of makefile pieces), which you then invoke with make or make install.

I’m surprised your traceback doesn’t get more informative. Here’s what a traceback entry looks like for me after a debug build:

frame #0: 0x000000010031be08 CDMS_G4DMC`G4DMChitSD::ProcessHits(this=0x0000000111e73940, aStep=0x0000000111e4b050, ROhist=0x0000000000000000) at G4DMChitSD.cc:173

It shows the function arguments (in this case they’re pointers, not values), and the source code line number.

However, I’m glad that you figured out what the problem was!