You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "Pugachev Maxim (JIRA)" <ji...@apache.org> on 2012/05/16 12:41:02 UTC

[jira] [Created] (AVRO-1092) avro-c: improving thread safety in error management code

Pugachev Maxim created AVRO-1092:
------------------------------------

             Summary: avro-c: improving thread safety in error management code
                 Key: AVRO-1092
                 URL: https://issues.apache.org/jira/browse/AVRO-1092
             Project: Avro
          Issue Type: Bug
          Components: c
    Affects Versions: 1.6.3, 1.7.0
            Reporter: Pugachev Maxim
            Priority: Critical


Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.

Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Pugachev Maxim (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pugachev Maxim updated AVRO-1092:
---------------------------------

    Attachment: AVRO-1092.patch

Patch for this issue.
                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>         Attachments: AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Pugachev Maxim (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13277626#comment-13277626 ] 

Pugachev Maxim commented on AVRO-1092:
--------------------------------------

> 1. Are THREAD_LIBRARIES and THREADSAFE cmake intrinsic definitions or are they your definitions?

This is mine definitions.


> 2. I didn't see why the definition _REENTRANT was set. It isn't used anywhere in the source. Is it a requirement of pthreads?

Defining _REENTRANT causes the compiler to use thread safe (i.e. re-entrant) versions of several functions in the C library. This is not a requirement, but it should be used in MT code.


> 3. How do you disable or enable threads (under Linux)?

You need to pass a THREADSAFE flag to cmake: "cmake -DTHREADSAFE=true ."


> Is there a reason you didn't use the syntax similar to the zlib and lzma codecs?

There is no flag named "Threads_FOUND" in cmake`s module FindThreads.cmake. In perfect scenario this code should be like this:

if(THREADSAFE)
    find_package(Threads)
    add_definitions(-DTHREADSAFE -D_REENTRANT)
    set(THREADS_LIBRARIES ${CMAKE_THREAD_LIBS_INIT})

    #check for Unix/Win32 and add extra definitions
endif(THREADSAFE)

${CMAKE_THREAD_LIBS_INIT} contains correct thread library (pthreads or win32 threads).
                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>         Attachments: AVRO-1092-patch-2.patch, AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Douglas Creager (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13279776#comment-13279776 ] 

Douglas Creager commented on AVRO-1092:
---------------------------------------

Looks good except for one detail — we don't have to add {{-pthread}} to the compiler flags since FindThreads will add that for us if it's needed on the current platform.
                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>         Attachments: AVRO-1092-patch-2.patch, AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[jira] [Commented] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Vivek Nadkarni (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13277137#comment-13277137 ] 

Vivek Nadkarni commented on AVRO-1092:
--------------------------------------

A few questions regarding the patch, specifically the CMakeLists.txt
file. I am by no means a CMake expert, so apologies if the answers
should be obvious :-).

1. Are THREAD_LIBRARIES and THREADSAFE cmake intrinsic definitions or
   are they your definitions?

2. I didn't see why the definition _REENTRANT was set. It isn't used
   anywhere in the source. Is it a requirement of pthreads?

3. How do you disable or enable threads (under Linux)? Is there a
   reason you didn't use the syntax similar to the zlib and lzma
   codecs? For example

    find_package(Threads)
    if (Threads_FOUND)
      message("Threads_FOUND")
      # Use threads here.
    else (Threads_FOUND)
      message("Threads not FOUND")
    endif(Threads_FOUND)


Thanks,
Vivek
                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>         Attachments: AVRO-1092-patch-2.patch, AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Douglas Creager (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13279779#comment-13279779 ] 

Douglas Creager commented on AVRO-1092:
---------------------------------------

Oh and I described how to set the {{THREADSAFE}} cmake variable in the INSTALL file.
                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>             Fix For: 1.7.0
>
>         Attachments: AVRO-1092-patch-2.patch, AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Vivek Nadkarni (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Nadkarni updated AVRO-1092:
---------------------------------

    Attachment: AVRO-1092-patch-2.patch

I downloaded the patch to test it out in Windows, since you had called
out the potential Windows incompatibility. The code didn't compile
under Windows, but I was able to get the code to compile and pass
tests with a minor modification. 

I am attaching my minor modification as a patch that should be applied
on top of AVRO-1092.patch. This patch should be applied from within
the avro-trunk/lang/c directory.

My patch also updates the file README.maintaining_win32.txt, to
capture the inability of MSVC++ to support structure initialization
using element names.


                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>         Attachments: AVRO-1092-patch-2.patch, AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Vivek Nadkarni (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13278201#comment-13278201 ] 

Vivek Nadkarni commented on AVRO-1092:
--------------------------------------

Thanks for the clarification. 

Cheers,
Vivek
                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>         Attachments: AVRO-1092-patch-2.patch, AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Douglas Creager (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Douglas Creager updated AVRO-1092:
----------------------------------

       Resolution: Fixed
    Fix Version/s: 1.7.0
           Status: Resolved  (was: Patch Available)

Committed to SVN with Vivek's Windows-compatibility updates, and without setting {{-pthread}} compiler flag manually.
                
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>             Fix For: 1.7.0
>
>         Attachments: AVRO-1092-patch-2.patch, AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (AVRO-1092) avro-c: improving thread safety in error management code

Posted by "Pugachev Maxim (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-1092?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pugachev Maxim updated AVRO-1092:
---------------------------------

    Status: Patch Available  (was: Open)
    
> avro-c: improving thread safety in error management code
> --------------------------------------------------------
>
>                 Key: AVRO-1092
>                 URL: https://issues.apache.org/jira/browse/AVRO-1092
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.3, 1.7.0
>            Reporter: Pugachev Maxim
>            Priority: Critical
>         Attachments: AVRO-1092.patch
>
>
> Error management code isn`t thread safe at all. I wrote a patch for this issue, but it works only for *nix systems.
> Affected functions: avro_set_error(), avro_prefix_error()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira