Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-3455

IPC.Client synchronisation looks weak

VotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.18.0
    • 0.18.0
    • ipc
    • None
    • Reviewed

    Description

      Looking at HADOOP-3453 , its clear that Client.java is inconsistently synchronized

      1. the running and shouldCloseConnection flags are not always read/written in synchronized blocks, even though they are properties used to share information between threads. They should be marked as volatile for access outside synchronized blocks, and all read-check-update operations must be synchronized.

      2. there are multiple calls to System.currentTimeMillis() in synchronized blocks; this is a slow native operation and should ideally be done unsynchronized.

      3. Synchronizing on the (out) stream is dangerous as its value changes during the life of the class, and sometimes it is null. These blocks should all synchronize on the Client instead.

      4. There are a number of places where InterruptedExceptions are caught and ignored in a sleep-wait loop:
      } catch (InterruptedException e) {
      }

      This isn't dangerous, but it does make the client harder to stop. These code fragments should be looked at carefully.

      Attachments

        1. hadoop-3455.patch
          11 kB
          Steve Loughran
        2. hadoop-3455-updated.patch
          10 kB
          Steve Loughran
        3. ipcClient.patch
          11 kB
          Hairong Kuang
        4. ipcClient1.patch
          13 kB
          Hairong Kuang

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            hairong Hairong Kuang
            stevel@apache.org Steve Loughran
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment