STEPControl_Reader Access violation reading location

martijn db
Wed, 10/09/2024 - 12:32

Forums:

Good morning,

I'm encountering an issue with the STEPControl_Reader in my program:

The program starts by creating a concurrent queue. The queue items consists of file paths. After the queue has been created, i create 16 worker threads that consume the queue items. The work each thread does, consists of reading a stepfile using STEPControl_Reader, as well as some further processing.

Unfortunately the program *sometimes* crashes at this line of code: "STEPControl_Reader reader;"
It doesn't only crash when reading a specific step file; Its a different file every time it crashes.

-The stepfiles are monoparts (so not assemblies.)
-The program reads approximately 2,800 STEP files (about 290 MB in total).
-Sometimes it crashes after reading a few hundred files, sometimes the program is able to process all files a few times before crashing.
-Opencascade version 7.8.1

This is how I'm currently using the STEPControl_Reader class:

TopoDS_Shape FileReader::ReadStepFileFromPath(const std::string& path)
{
if (!std::filesystem::exists(path))
{
std::cout << "File " << path << " does not exist" << "\n";
return {};
}

STEPControl_Reader reader; //<- Crash on this line
IFSelect_ReturnStatus valid = reader.ReadFile(path.c_str());
if (valid != IFSelect_RetDone)
{
std::cout << "Error reading file: " << path << "\n";
return {};
}

return TransferReader(reader);
}

TopoDS_Shape FileReader::TransferReader(STEPControl_Reader& reader)
{
try
{
Standard_Integer NbRoots = reader.NbRootsForTransfer();
Standard_Integer NbTrans = reader.TransferRoots();

TopoDS_Shape result = reader.OneShape();
if (result.IsNull())
return {};

return result;
}
catch (...)
{
std::cout << "Error processing reader (probably STEP not valid) \n";
return {};
}
}

I've attached the call stack, as well as the error message.

Am I using the step STEPControl_Reader incorrectly? Or is there a bug inside of the opencascade code?
If you need more information, let me know!

Thanks in advance!

Martijn

Attachments:

Capture.PNG

Capture2.PNG

Capture3.PNG

Dmitrii Pasukhin
Wed, 10/09/2024 - 12:42

Hello. You reading correctly.

I can't help you. The information is not enough. Did you read in multithreading? Or in single thread.

Best regards, Dmitrii.

martijn db
Wed, 10/09/2024 - 12:46

Hi Dmitrii,

I'm reading in multithreading. Please let me know if you need more information!

Martijn

Dmitrii Pasukhin
Wed, 10/09/2024 - 13:05

The issue is there. My apologies. I detect the multithreading issue. But create a workaround on specific branch.

The issue on parsing stage. And it is not related with the amount of files that were read. I can create a mutex on that critical operation as temp workaround. The stable solution is more complicated and will takes time. I'm sorry about that.

Temp path will be applicable for you?

Best regards, Dmitrii.

martijn db
Wed, 10/09/2024 - 13:24

Hi dmitrii,

Many thanks for your fast reponses and research!

I would love to try the temp workaround!

About the stable solution, do you have an estimation on the time it will take? Will it be released in version 7.8.2 or 7.9 (or later)?

Martijn

Dmitrii Pasukhin
Wed, 10/09/2024 - 13:36

I hope it will on 7.9. the main point it is required for one of our customers.

I will try to increase the priority on that.

As for a workaround. I will share it during today. If I will lost, please send a reminder as a new commit. I have notification for all forum activities on my mail

Best regards, Dmitrii.

Dmitrii Pasukhin
Wed, 10/09/2024 - 19:57

Hello,

There are a small patch. You can use git apply or insert manually

diff --git a/src/StepFile/StepFile_Read.cxx b/src/StepFile/StepFile_Read.cxx
index 26319a7c43..b176a70264 100644
--- a/src/StepFile/StepFile_Read.cxx
+++ b/src/StepFile/StepFile_Read.cxx
@@ -30,6 +30,7 @@
 #include <StepData_StepReaderTool.hxx>
 
 #include <Standard_ErrorHandler.hxx>
+#include <Standard_Mutex.hxx>
 #include <Standard_Failure.hxx>
 
 #include <Message.hxx>
@@ -62,6 +63,8 @@ static Standard_Integer StepFile_Read (const char* theName,
                                        const Handle(StepData_FileRecognizer)& theRecogHeader,
                                        const Handle(StepData_FileRecognizer)& theRecogData)
 {
+  static Standard_Mutex aMutex;
+  Standard_Mutex::Sentry aLocker(aMutex);
   // if stream is not provided, open file stream here
   std::istream* aStreamPtr = theIStream;
   std::shared_ptr<std::istream> aFileStream;

As for a StepControl_Reader I suggest to do the next:

static std::mutex aMutex;
aMutex.lock();
STEPControl_Reader aReader;
aMutex.unlock();

Best regards, Dmitrii.

martijn db
Wed, 10/09/2024 - 21:02

Hi Dmitrii,

Thanks for the quick patch.
If i apply the "StepFile_Read.cxx" patch, do I also need to use the code below?

static std::mutex aMutex;
aMutex.lock();
STEPControl_Reader aReader;
aMutex.unlock();

Or is implementing one of the solutions sufficient?

Martijn

Dmitrii Pasukhin
Wed, 10/09/2024 - 21:45

You need both. First for actor read constriction.

Second for parsing.

I put into muted only constructor. Nothing more.

Best regards, Dmitrii.

martijn db
Fri, 11/01/2024 - 16:20

Hello,

I have noticed the same issue with the StepControl_Writer.

IFSelect_ReturnStatus StepWriter::WriteShape(const TopoDS_Shape& shape, const std::string& path)
{
	STEPControl_Writer writer;
	IFSelect_ReturnStatus transferStatus = writer.Transfer(shape, STEPControl_AsIs);
	if (transferStatus != IFSelect_RetDone)
		return transferStatus;

	IFSelect_ReturnStatus writeStatus = writer.Write(path.c_str());
	if (writeStatus != IFSelect_RetDone)
		return writeStatus;

	return IFSelect_RetDone;
}

More specifically, "writer.write()" sometimes gives me this error when I write multiple files in parallel:
Exception thrown at 0x00007FF8ADD0F320 (TKDESTEP.dll) in project.exe: 0xC0000005: Access violation reading location 0x0000000000000010.

Is the stepwriter thread safe? Or am I doing something wrong?

Dmitrii Pasukhin
Sat, 11/02/2024 - 13:19

Hello. You are using not thread safety interface. There can be issue with constructor and Transfer method. Please check Transfer method implementation with StepData_ConfParameters usage.

For now multithreading is working only with that conditionals. I have a projects that have a thousands of import/export operation per day in multithread.

But for that, needs to apply changes for the Reader(see first comment) and put mutex for writer constructor and use Transfer with Parameters directly (only 7.8+).

The increase thread-safуty is one of the priority task for now. But without some mutexes is a little complicated in case of old OCCT architecture.

Best regards, Dmitrii.

martijn db
Tue, 11/19/2024 - 17:14

Hello Dmitrii,

I wanted to check in on the progress of the thread safety fix. Do you have any updates?
Do you still believe it will be achievable to include it in OCCT 7.9?
Additionally, could you share an estimated release date for this version?

Thanks in advance.

Martijn

Dmitrii Pasukhin
Tue, 11/19/2024 - 19:46

Hello. I can´t share the timeline for now. Depending on the date of 7.9 release it can be included. It is a commercial request, which is now done by workaround. So, the fix is on top of priority for maintaining development.

Best regards, Dmitrii.

martijn db
Wed, 11/20/2024 - 12:00

Hi Dmitrii,

Thanks for your response, I am looking forward to the update!

I am working on implementing your workaround. I think my reader works, but the writer still crashes.
This is what I am doing:

StepWriter:

static std::mutex writerMutex;

IFSelect_ReturnStatus StepWriter::WriteShape(const TopoDS_Shape& shape, const std::string& path)
{
	writerMutex.lock();
	STEPControl_Writer writer;
	writerMutex.unlock();

	StepData_ConfParameters conf;
	IFSelect_ReturnStatus transferStatus = writer.Transfer(shape, STEPControl_AsIs, conf);

	if (transferStatus != IFSelect_RetDone)
		return transferStatus;

	IFSelect_ReturnStatus writeStatus = writer.Write(path.c_str());

	return writeStatus;
}

StepReader:

static std::mutex readerMutex;
TopoDS_Shape FileReader::ReadStepFileFromPath(const std::string& path)
{
	if (!std::filesystem::exists(path))
	{
		std::cout << "File " << path << " does not exist" << "\n";
		return {};
	}
	readerMutex.lock();
	STEPControl_Reader reader;
	readerMutex.unlock();

	IFSelect_ReturnStatus valid = reader.ReadFile(path.c_str());
	if (valid != IFSelect_RetDone)
	{
		std::cout << "Error reading file: " << path << "\n";
		return {};
	}

	return TransferReader(reader);
}

I put the mutexes initialization in the class scope because I have multiple reader/writer methods.

As you can see, I added the StepData_ConfParameters to the writer.
I have also added the standard mutex to StepFile_Read.cxx.

Do I also need to add a standard mutex to one of the opencascade stepwriter files?

Martijn