axis-c-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carsten Blecken" <cblec...@macrovision.com>
Subject RE: Possbile bug in Axis.cpp on Win32 (XP, VC7.1)
Date Fri, 22 Apr 2005 18:35:31 GMT
Volatile didn't seem to make a difference, but I think the warning is inconsequential.
 
I moved this thread into AXISCPP-629 and will check it in soon.
 
BTW, is there a way to get me more permissions on JIRA like assigning bugs,
closing them etc.?
 
Carsten

-----Original Message-----
From: Tim Bartley [mailto:tbartley@au1.ibm.com]
Sent: Thursday, April 21, 2005 6:08 PM
To: Apache AXIS C Developers List
Cc: Apache AXIS C Developers List
Subject: RE: Possbile bug in Axis.cpp on Win32 (XP, VC7.1)



I couldn't find a JIRA to post  the suggested fix against - by posted I just meant to the
axis-c-dev list  (in fact this thread I believe) - should've opened a JIRA I guess. If you
get the (volatile) warnings on VC 7.1 then you can probably just remove the volatile key word
- I don't think 6 uses it either. 

Why are initialize_module/uninitialize_module extern "C" anyway? They are bad function names
to be exported by the shared library. Is this for the C binding support? I'd suggest that
if these aren't simply meant to be local functions to Axis.cpp (which appears to be how they're
used) then a better structure would be: 

static int initialize_module(int bServer) throw AxisException { /* existing code */ } 
static int uninitialize_module() throw AxisException { /* existing code */ } 

extern "C" int axisc_initialize_module(int bServer) { 
        try { 
                return initialize_module(bServer); 
        } 
        catch (AxisException& e) { 
                /* log error */ 
                return AXIS_FAILURE; 
        } 
        catch (...) { 
                /* log error */ 
                return AXIS_FAILURE; 
        } 
} 

extern "C" int axisc_uninitialize_module() { 
        try { 
                retrurn uninitialize_module(); 
        } 
        catch (AxisException& e) { 
                /* log error */ 
                return AXIS_FAILURE; 
        } 
        catch (...) {
               /* log error */ 
                return AXIS_FAILURE; 
        } 
} 

Cheers, 

Tim
--
IBM Tivoli Access Manager Development
Gold Coast Development Lab, Australia
+61-7-5552-4001 phone
+61-7-5571-0420 fax 



"Carsten Blecken" <cblecken@macrovision.com> 


04/22/05 10:52 AM 


Please respond to
"Apache AXIS C Developers List"



To
"Apache AXIS C Developers List" <axis-c-dev@ws.apache.org> 

cc

Subject
RE: Possbile bug in Axis.cpp on Win32 (XP, VC7.1)

	




Whoops you got me there. The mutex has to be created single threaded, which 
doesn't apply here. If you say that you posted the code is there a JIRA number 
for it (other than 557). 
  
I had to add some explicit casts to get it compile on VC7.1 

static volatile long g_uModuleInitializing = 0; 
static void start_initializing() 
{ 
   while (InterlockedIncrement((LONG*)&g_uModuleInitializing) != 1) { 
       InterlockedDecrement((LONG*)&g_uModuleInitializing); 
   } 
} 
static void done_initializing() 
{ 
   InterlockedDecrement((LONG*)&g_uModuleInitializing); 
}

And we might want to move away from the /EHc compile flag:

       [cc] c:\eclipse301c\eclipse\workspace\axisc_head\src\engine\Axis.cpp(463)
: warning C4297: 'initialize_module' : function assumed not to throw an excepti
on but does
      [cc]         The function is extern "C" and /EHc was specified
      [cc] c:\eclipse301c\eclipse\workspace\axisc_head\src\engine\Axis.cpp(463)
: warning C4297: 'initialize_module' : function assumed not to throw an excepti
on but does
      [cc]         The function is extern "C" and /EHc was specified
      [cc] c:\eclipse301c\eclipse\workspace\axisc_head\src\engine\Axis.cpp(498)
: warning C4297: 'uninitialize_module' : function assumed not to throw an excep
tion but does
      [cc]         The function is extern "C" and /EHc was specified
      [cc] c:\eclipse301c\eclipse\workspace\axisc_head\src\engine\Axis.cpp(498)
: warning C4297: 'uninitialize_module' : function assumed not to throw an excep
tion but does 


Carsten 



-----Original Message-----
From: Tim Bartley [mailto:tbartley@au1.ibm.com]
Sent: Thursday, April 21, 2005 5:05 PM
To: Apache AXIS C Developers List
Subject: RE: Possbile bug in Axis.cpp on Win32 (XP, VC7.1)




The purpose of the code is to avoid the (very small) risk of multiple threads racing to initialise.


Two threads racing through the code below can both simultaneously pass the "if (NULL == hMutex)"
test - both create separate mutexes - and both successfully lock it - then crash horribly
while they race through the initialization code. 

I have already posted the code that should compile on both VC6 and VC7 - has no body applied
this to the tree yet? 

Here is the modified code: 

static volatile long g_uModuleInitializing = 0; 
static void start_initializing() 
{ 
   while (InterlockedIncrement(&g_uModuleInitializing) != 1) { 
       InterlockedDecrement(&g_uModuleInitializing); 
   } 
} 
static void done_initializing() 
{ 
   InterlockedDecrement(&g_uModuleInitializing); 
} 

or maybe this has been applied but doesn't compile because of the volatile/non-volatile mismatch
between VC6 and VC7? I haven't seen the latest CVS source. 

Cheers, 

Tim
--
IBM Tivoli Access Manager Development
Gold Coast Development Lab, Australia
+61-7-5552-4001 phone
+61-7-5571-0420 fax 



"Carsten Blecken" <cblecken@macrovision.com> 


04/22/05 09:58 AM 



Please respond to
"Apache AXIS C Developers List"




To
"Apache AXIS C Developers List" <axis-c-dev@ws.apache.org> 

cc

Subject
RE: Possbile bug in Axis.cpp on Win32 (XP, VC7.1)


	





This seems to be the main reason why we can't compile on VC7.

I looked at the defs in winbase.h in 6 and 7 and the arguments have really
changed. Must be still void* in the end, since the call is implemented in 
kernel32.dll.

Apart from defining ifdefs (i.e. WIN32_VC6 and WIN32_VC7) I can only 
suggest to change the synchronization primitive. Why not using a mutex
like the UNIX code?

This seems to compile on both VC6 and VC7:
Axis.cpp(295)

#ifdef WIN32

static HANDLE hMutex = NULL; 

static void start_initializing()
{
               if (NULL == hMutex)
                                hMutex = CreateMutex( NULL, FALSE, NULL);

               WaitForSingleObject(hMutex, INFINITE);
}

static void done_initializing()
{
               ReleaseMutex(hMutex);
}

#else

Carsten


-----Original Message-----
From: Adrian Dick [mailto:adrian.dick@uk.ibm.com]
Sent: Monday, April 11, 2005 6:58 AM
To: Apache AXIS C Developers List
Subject: RE: Possbile bug in Axis.cpp on Win32 (XP, VC7.1)


Rob,

As you comment below, the code in CVS does not exactly match the original
patch provided, due to the apparent differences in API between MS VC++
versions.

>From <MS VC++ 6.0 install>\Include\WINBASE.H
 PVOID
 WINAPI
 InterlockedCompareExchange (
     PVOID *Destination,
     PVOID Exchange,
     PVOID Comperand
     );

I initially tried using the version provided in the original patch (which
matches your suggested usage), but received compilation errors, basically
the reverse of those you are seeing.

Adrian
_______________________________________
Adrian Dick (adrian.dick@uk.ibm.com)

"Rob Lievaart" <Rob.Lievaart@nob.nl> wrote on 11/04/2005 14:22:45:

> Hi Adrian,
>
> There are two things here, one is the signature of the function,
> and one is the actual parameter values.
>
> I can imagine something about the different compiler versions
> being more or less picky about signatures which types can and cannot be
> converted to each other, but I find it hard to believe  that
> Microsoft changed the actual system call.  According to the
> documentation parameter 2 and 3 need to be the values to be
> compared and exchanged, but instead of these values, _pointers_
> to these values are passed.
>
> As far as I can see, the current function passes two pointers instead
> of a '1' and a '0' for exchange and comparand.  The result is that
> g_uModuleInitializing is never equal to the comparand, so it is never
> actually set to any other value then 0. The function will return the
> initial 0,  so it will continue thinking it has the lock, but without
> actually setting the flag.
>
> I guess, the chance of multiple threads actually trying to initialize at
>
> the same time, is pretty low, so it will seem to work, but it does not
> provide any protection this way.
>
> > This appears to be a problem between different versions of MS VC++.
> >
> > I applied this fix, as provided in
> > http://issues.apache.org/jira/browse/AXISCPP-557.
>
> Ehh, the patch you refer to in this issue shows the following code:
>
> + static void start_initializing()
> + {
> +     while (InterlockedCompareExchange(&g_uModuleInitializing, 1, 0));
> + }
>
> However the version in CVS has:
>
> static void start_initializing()
> {
>     long exchange  = 1;
>     long comperand = 0;
>     while (InterlockedCompareExchange(((void **)&g_uModuleInitializing),
> void*)&exchange, (void *) &comperand)));
> }
>
> Apart from the void * casts, the first one passes the values 1 and 0,
> and the
> second one passes the address of 2 variables that contain 1 and zero.
> The version in CVS definately does not match de documentation from
> Microsoft.
>
> I currently don't have VC6.0 installed to check out, what the problem
> is. But my
> guess is that the version in the original patch is correct.
> (I used the original version on VC7.1 and it compiles)
>
> Do you remember what problems VC6.0 had with the original version?
>
>
> Kind regards,
>
>
>
> Rob.
>
>
>





Mime
View raw message