Jump to content

  • Log In with Google      Sign In   
  • Create Account

Jason Goepel

Member Since 15 May 2013
Offline Last Active Today, 08:25 AM

Posts I've Made

In Topic: Multithreading - Creating a Module

11 December 2014 - 11:53 AM

Thanks for the quick response, but I think there may still be a slight issue.  It seems like if two threads enter this block:

ACQUIRESHARED(engineRWLock);
if( lastModule && lastModule->name == name )
    retModule = lastModule;
else
{
    // TODO: optimize: Improve linear search
    for( asUINT n = 0; n < scriptModules.GetLength(); ++n )
        if( scriptModules[n] && scriptModules[n]->name == name )
        {
            lastModule = retModule = scriptModules[n];
            break;
        }
}
RELEASESHARED(engineRWLock);

 

Then there is the opportunity for:

Thread1:  "lastModule->name == name"    true

Thread2:  "scriptModules[n]->name == name"  true

Thread2:  "lastModule = retModule = scriptModules[n]"

Thread1:  "retModule = lastModule"

 

If that were to happen, then Thread1 would return the incorrect module.  It seems like "lastModule" should always be within an exclusive lock when being written to.


In Topic: Multithreading - Creating a Module

10 December 2014 - 07:42 AM

Sorry.  I thought I had uploaded it.


In Topic: Multithreading - Creating a Module

09 December 2014 - 01:00 PM

I think I found a gap in your solution.

asCModule *asCScriptEngine::GetModule(const char *_name, bool create)
{
    // Accept null as well as zero-length string
    const char *name = "";
    if( _name != 0 ) name = _name;

    if( lastModule && lastModule->name == name )
        return lastModule;

    lastModule = 0;
    ACQUIRESHARED(engineRWLock);
    // TODO: optimize: Improve linear search
    for( asUINT n = 0; n < scriptModules.GetLength(); ++n )
        if( scriptModules[n] && scriptModules[n]->name == name )
        {
            lastModule = scriptModules[n];
            break;
        }
    RELEASESHARED(engineRWLock);
    if( lastModule ) 
        return lastModule;

    if( create )
    {
        asCModule *module = asNEW(asCModule)(name, this);
        if( module == 0 )
        {
            // Out of memory
            return 0;
        }

        ACQUIREEXCLUSIVE(engineRWLock);
        scriptModules.PushLast(module);
        RELEASEEXCLUSIVE(engineRWLock);

        lastModule = module;

        return lastModule;
    }

    return 0;
}

The member variable "lastModule" should be guarded under an exclusive lock when writing.  I had one user experience a crash from a module being destroyed twice.  The problem was two threads were attempting to create new modules simultaneously.  One of the threads searched through the "scriptModules" array an found no matching module, while the other thread created a new module and saved it to "lastModule".  Then the first thread tested "lastModule" (immediately after the search), and it was not null (even though the search was not successful).  So both function calls returns the same "asIModule" pointer, setting the stage for the crash.

 

I have attached a patch with my solution.

 


In Topic: Integer Division

05 September 2014 - 01:55 PM

It turned out to be a very small compiler modification.  As far as I can tell, this is complete.


In Topic: Integer Division

04 September 2014 - 03:11 PM

I agree that explicit conversions would be more readable than a backslash.  The only issue with int(expr) would be that not all large, 64-bit numbers convert to doubles without loss, so the result of a floating-point division followed by truncation would not necessarily be the same as integer division.  I'm not too concerned about that though, because I can always create some sort of "idivide" function in my application. 


PARTNERS