[jira] Created: (NET-220) TFTPServer is not threadsafe

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (NET-220) TFTPServer is not threadsafe

JIRA jira@apache.org
TFTPServer is not threadsafe
----------------------------

                 Key: NET-220
                 URL: https://issues.apache.org/jira/browse/NET-220
             Project: Commons Net
          Issue Type: Bug
    Affects Versions: 2.0
            Reporter: Sebb


The TFTPServer class is not threadsafe.

There are several reasons for this:

Ctors call private launch() method which creates a thread and start()s it.
This publishes the instance before it has been fully constructed.

Various instance fields are not thread-safe - e.g. serverReadDirectory_ is neither final nor volatile, and accesses to it are not synchronized.
Although it is only written by the ctors, the lack of synch means that the field may not be visible to other threads.

As far as possible, instance fields should be made final - this guarantees that the field will be visible to other threads.

One of the shutdown_ variables is volatile (so is threadsafe) but the shutdown_ variable in the nested TFTPTransfer class is not,  yet is accessed from multiple threads.

The access to the variable serverException is not synch; should probably be volatile.

Patch to follow.


--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (NET-220) TFTPServer is not threadsafe

JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/NET-220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12597990#action_12597990 ]

Sebb commented on NET-220:
--------------------------

Just realised that fixing the launch() problem will require a change to the API.

The server needs to be started by the caller; this will require changes to Test code.

There should be no existing application code, as the class is new to 2.0 - as far as I know.

> TFTPServer is not threadsafe
> ----------------------------
>
>                 Key: NET-220
>                 URL: https://issues.apache.org/jira/browse/NET-220
>             Project: Commons Net
>          Issue Type: Bug
>    Affects Versions: 2.0
>            Reporter: Sebb
>
> The TFTPServer class is not threadsafe.
> There are several reasons for this:
> Ctors call private launch() method which creates a thread and start()s it.
> This publishes the instance before it has been fully constructed.
> Various instance fields are not thread-safe - e.g. serverReadDirectory_ is neither final nor volatile, and accesses to it are not synchronized.
> Although it is only written by the ctors, the lack of synch means that the field may not be visible to other threads.
> As far as possible, instance fields should be made final - this guarantees that the field will be visible to other threads.
> One of the shutdown_ variables is volatile (so is threadsafe) but the shutdown_ variable in the nested TFTPTransfer class is not,  yet is accessed from multiple threads.
> The access to the variable serverException is not synch; should probably be volatile.
> Patch to follow.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (NET-220) TFTPServer is not threadsafe

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/NET-220?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sebb updated NET-220:
---------------------

    Attachment: TFTPServerPathTest.patch
                TFTPServer.patch

Patch to TFTPServer changes all instance variables to final or volatile.

There did not seem to be a use-case for changing the logging destinations after construction, so rather than making these volatile (or adding synchronisation) I removed the set() methods.

The method used to start the server is called start(), but could of course be renamed if required.

> TFTPServer is not threadsafe
> ----------------------------
>
>                 Key: NET-220
>                 URL: https://issues.apache.org/jira/browse/NET-220
>             Project: Commons Net
>          Issue Type: Bug
>    Affects Versions: 2.0
>            Reporter: Sebb
>         Attachments: TFTPServer.patch, TFTPServerPathTest.patch
>
>
> The TFTPServer class is not threadsafe.
> There are several reasons for this:
> Ctors call private launch() method which creates a thread and start()s it.
> This publishes the instance before it has been fully constructed.
> Various instance fields are not thread-safe - e.g. serverReadDirectory_ is neither final nor volatile, and accesses to it are not synchronized.
> Although it is only written by the ctors, the lack of synch means that the field may not be visible to other threads.
> As far as possible, instance fields should be made final - this guarantees that the field will be visible to other threads.
> One of the shutdown_ variables is volatile (so is threadsafe) but the shutdown_ variable in the nested TFTPTransfer class is not,  yet is accessed from multiple threads.
> The access to the variable serverException is not synch; should probably be volatile.
> Patch to follow.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (NET-220) TFTPServer is not threadsafe

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/NET-220?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sebb updated NET-220:
---------------------

    Component/s: TFTP

> TFTPServer is not threadsafe
> ----------------------------
>
>                 Key: NET-220
>                 URL: https://issues.apache.org/jira/browse/NET-220
>             Project: Commons Net
>          Issue Type: Bug
>          Components: TFTP
>    Affects Versions: 2.0
>            Reporter: Sebb
>         Attachments: TFTPServer.patch, TFTPServerPathTest.patch
>
>
> The TFTPServer class is not threadsafe.
> There are several reasons for this:
> Ctors call private launch() method which creates a thread and start()s it.
> This publishes the instance before it has been fully constructed.
> Various instance fields are not thread-safe - e.g. serverReadDirectory_ is neither final nor volatile, and accesses to it are not synchronized.
> Although it is only written by the ctors, the lack of synch means that the field may not be visible to other threads.
> As far as possible, instance fields should be made final - this guarantees that the field will be visible to other threads.
> One of the shutdown_ variables is volatile (so is threadsafe) but the shutdown_ variable in the nested TFTPTransfer class is not,  yet is accessed from multiple threads.
> The access to the variable serverException is not synch; should probably be volatile.
> Patch to follow.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.