PavelBlend / blender-molecular

molecular addon for blender
26 stars 7 forks source link

Access violation writing location #11

Closed OmidGhotbi closed 5 months ago

OmidGhotbi commented 5 months ago

Exception thrown at 0x00007FFF6B2F4A2F (core.cp310-win_amd64.pyd) in blender.exe: 0xC0000005: Access violation writing location 0x00007FF5D5445748. most crashes are coming from malloc, if you help me how can I attach the visual studio to compile symbols for core.cp310-win_amd64.pyd I can help you to fixed the issues

PavelBlend commented 5 months ago

@OmidGhotbi in the last commit I added debug compilation. You need to set the DEBUG variable to True.

OmidGhotbi commented 5 months ago

i already did like that:

module = distutils.extension.Extension(
    'core',
    sources=['core\\main.c'],
    extra_compile_args=['/openmp', '/GT', '/fp:fast', '/Zi'] #/Ox optimization Or /Zi debug
)

and I get core.cp310-win_amd64.pyd and vc140.pdb

then attached vscode to blender with debugger attach 
{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "name": "(Windows) Attach",
            "type": "cppvsdbg",
            "request": "attach",
            "processId": "33212", // Blender PID to be attached to debuger
            "symbolSearchPath": "${workspaceFolder}" // path to symbols
        }
    ]
}

I can see the memory violation but it's not giving me any more data that is attached to the blender PID itself, also, break point on C code not working too. or on error I can not see the error happened in which part of the code. Am I doing something wrong? or do I need to do something to get more debug information that's my problem.

By the way, I found a few memory leak issues and parallel conflicts,i t's working way more stable , should i create a pull request or create a new GitHub for it?

PavelBlend commented 5 months ago

should i create a pull request

create a pull request

Maybe it's worth trying to remove all OpenMP directives? Check the code for functionality without parallelism. Start fixing errors and if everything works stably, then you can enable OpenMP and start fixing parallel code.

It seems to me that most of the errors are related to OpenMP loops.

Am I doing something wrong? or do I need to do something to get more debug information that's my problem.

I am not qualified in this field. I don't know what to do.

OmidGhotbi commented 5 months ago

As you already mentioned most problems come from Multithread parallelism, memory allocation and reallocation, the simulation is not thread-safe so once a while one thread will overwrite the variable that should not when the other thread still needs that variable data. i created a fail-safe manual try cache for mallocate a relocate and also removed some parallelism and now it works 10 times more stable with almost the same performance. we can work on performance later.

PavelBlend commented 5 months ago

Why did the Cython code work without failure? Did the Cython compiler automatically generate thread-safe code? I rewrote the code from Cython to C as is, without changes.

OmidGhotbi commented 5 months ago

GIL (Global Interpreter Lock) Management: Cython provides a way to release and acquire the GIL around specific blocks of code, which can make multithreaded code safer and easier to write. even when you use nogil is still inside the safe mechanism som how like another thread make it safe. Type Inference and Checking: Cython performs static type inference and checking, which can catch potential errors at compile time rather than at runtime. Cython will do a few things that in C you need to do it manually, in C++ you can handle them easily but in C they need to be addressed manually for example Try and cache, or type checking, and even STL and exception handling. C++ has stronger type checking which can prevent many programming errors that would go unnoticed in C. They adopt many of these aspects in Cython not all of them but enough to make it easier than C code. also openMP do a few tests and thread lock that is not automatic in C, they slow down the process a little bit but make it more stable.

OmidGhotbi commented 5 months ago

However the Cython code will face crash on the higher number of particles as well, and has a huge memory leak on relink especially when you use the break option in relink. i already patched it two days ago. os it wasn't that bullet prof either.

OmidGhotbi commented 5 months ago

I'll give you a few examples, about multithreading you write this code from Cython: int i = 0;

pragma omp parallel for schedule(dynamic, 10)

for (i=0; i<parnum; i++) {
    parlistcopy[i].id = parlist[i].id;
    parlistcopy[i].loc[0] = parlist[i].loc[0];
    parlistcopy[i].loc[1] = parlist[i].loc[1];
    parlistcopy[i].loc[2] = parlist[i].loc[2];
}

in Cython this is not the perfect implementation but somehow it is correct. but in C the 'i' variable will be shared between all threads and it will cause a race between threads. it can certainly cause a violation of memory error. the correct way to use it is like this:

pragma omp parallel

{
    int i;
    #pragma omp for schedule(dynamic, 10)
  for (i=0; i<parnum; i++) {
    parlistcopy[i].id = parlist[i].id;
    parlistcopy[i].loc[0] = parlist[i].loc[0];
    parlistcopy[i].loc[1] = parlist[i].loc[1];
    parlistcopy[i].loc[2] = parlist[i].loc[2];
   }
}

so 'i' is a variable that will be defined for every single thread and will not be shared between them.
OmidGhotbi commented 5 months ago

for memory allocate is the same story. if you use it in Cython because it will use python to compile the code in C it will fix the issue but in C most of compiler will do nothing for you some will notify. example:

Pool parPool = (Pool) malloc(sizeof(Pool));

if it returns null it will crash the app at the first time you use the parPool and you can not do anything about it. so the correct way to handle it will be :

Pool *parPool if (parPool== NULL) { fprintf(stderr, "Memory allocation failed for %s\n", var_name); exit(1); }

so if you reach null or out-of-bound memory allocation, the app will not crash whole and you can control it at least and return a respons.

OmidGhotbi commented 5 months ago

I forgot to tell you OpenMP has a bad reputation in nested loops, so if you have a loop that uses openMP and you have a loop inside that you probably going to have a crash in run time no matter what you do, using atomic or mutex can help a lot but the performance you will loos and the overhead will let you with more work, more CPU usage and not that much of performance.

OmidGhotbi commented 5 months ago

Fixed.