FAQ
Race condition around HRegionServer -> HMaster communication
------------------------------------------------------------

Key: HBASE-1873
URL: https://issues.apache.org/jira/browse/HBASE-1873
Project: Hadoop HBase
Issue Type: Bug
Affects Versions: 0.20.0
Reporter: Eric Tschetter


HRegionServer on lines 459 - 463 (part of run()) accesses outboundMsgs in a synchronized fashion, but other uses of the object are not synchronized.

Specifically, the code is

459 synchronized(this.outboundMsgs) {
460 outboundArray =
461 this.outboundMsgs.toArray(new HMsg[outboundMsgs.size()]);
462 this.outboundMsgs.clear();
463 }

Whereas things are added to this list from calls like

private void reportOpen(HRegionInfo region) {
outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, region));
}

And

void reportSplit(HRegionInfo oldRegion, HRegionInfo newRegionA,
HRegionInfo newRegionB) {
outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_SPLIT, oldRegion,
("Daughters; " +
newRegionA.getRegionNameAsString() + ", " +
newRegionB.getRegionNameAsString()).getBytes()));
outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionA));
outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionB));
}

It looks like the object is initialized as

private final List<HMsg> outboundMsgs =
Collections.synchronizedList(new ArrayList<HMsg>());

Which would appear to provide security, but it doesn't actually prevent an insert from happening between lines 461 and 462, which would subsequently get removed from the call to clear(). At least, from the Sun HotSpot source code, it looks like Collections.synchronizedList() does the right thing and synchronizes on an inner mutex object instead of synchronizing on the externally visible list object itself. That means, however, that the synchronized() on line 459 is largely meaningless.

I'm not sure how often this race condition would occur in the wild, but every thread waiting on the mutex around the toArray() call increases the probability that the next person to get the mutex is someone who wants to add something to the List, rather than the thread calling clear().

Simple fix would be to do external synchronization around all accesses to the List. Barring that, perhaps a SynchronizedList implementation with a "emptyToArray()" method that encapsulates the toArray() and subsequent clear().

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

Search Discussions

  • Eric Tschetter (JIRA) at Sep 29, 2009 at 1:58 am
    [ https://issues.apache.org/jira/browse/HBASE-1873?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

    Eric Tschetter updated HBASE-1873:
    ----------------------------------

    Description:
    HRegionServer on lines 459 - 463 (part of run()) accesses outboundMsgs in a synchronized fashion, but other uses of the object are not synchronized.

    Specifically, the code is

    459 synchronized(this.outboundMsgs) {
    460 outboundArray =
    461 this.outboundMsgs.toArray(new HMsg[outboundMsgs.size()]);
    462 this.outboundMsgs.clear();
    463 }

    Whereas things are added to this list from calls like

    private void reportOpen(HRegionInfo region) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, region));
    }

    And

    void reportSplit(HRegionInfo oldRegion, HRegionInfo newRegionA,
    HRegionInfo newRegionB) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_SPLIT, oldRegion,
    ("Daughters; " +
    newRegionA.getRegionNameAsString() + ", " +
    newRegionB.getRegionNameAsString()).getBytes()));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionA));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionB));
    }

    It looks like the object is initialized as

    private final List<HMsg> outboundMsgs =
    Collections.synchronizedList(new ArrayList<HMsg>());

    Which would appear to provide security, but it doesn't actually prevent an insert from happening between lines 461 and 462, which would subsequently get removed from the call to clear(). At least, from the Sun HotSpot source code, it looks like Collections.synchronizedList() does the right thing and synchronizes on an inner mutex object instead of synchronizing on the externally visible list object itself. That means, however, that the synchronized() on line 459 is largely meaningless.

    I'm not sure how often this race condition would occur in the wild, but every thread waiting on the mutex around the toArray() call increases the probability that the next person to get the mutex is someone who wants to add something to the List, rather than the thread calling clear().

    Simple fix would be to do external synchronization around all accesses to the List. Barring that, perhaps a SynchronizedList implementation with a "emptyToArray()" method that encapsulates the toArray() and subsequent clear().

    was:
    HRegionServer on lines 459 - 463 (part of run()) accesses outboundMsgs in a synchronized fashion, but other uses of the object are not synchronized.

    Specifically, the code is

    459 synchronized(this.outboundMsgs) {
    460 outboundArray =
    461 this.outboundMsgs.toArray(new HMsg[outboundMsgs.size()]);
    462 this.outboundMsgs.clear();
    463 }

    Whereas things are added to this list from calls like

    private void reportOpen(HRegionInfo region) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, region));
    }

    And

    void reportSplit(HRegionInfo oldRegion, HRegionInfo newRegionA,
    HRegionInfo newRegionB) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_SPLIT, oldRegion,
    ("Daughters; " +
    newRegionA.getRegionNameAsString() + ", " +
    newRegionB.getRegionNameAsString()).getBytes()));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionA));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionB));
    }

    It looks like the object is initialized as

    private final List<HMsg> outboundMsgs =
    Collections.synchronizedList(new ArrayList<HMsg>());

    Which would appear to provide security, but it doesn't actually prevent an insert from happening between lines 461 and 462, which would subsequently get removed from the call to clear(). At least, from the Sun HotSpot source code, it looks like Collections.synchronizedList() does the right thing and synchronizes on an inner mutex object instead of synchronizing on the externally visible list object itself. That means, however, that the synchronized() on line 459 is largely meaningless.

    I'm not sure how often this race condition would occur in the wild, but every thread waiting on the mutex around the toArray() call increases the probability that the next person to get the mutex is someone who wants to add something to the List, rather than the thread calling clear().

    Simple fix would be to do external synchronization around all accesses to the List. Barring that, perhaps a SynchronizedList implementation with a "emptyToArray()" method that encapsulates the toArray() and subsequent clear().

    Race condition around HRegionServer -> HMaster communication
    ------------------------------------------------------------

    Key: HBASE-1873
    URL: https://issues.apache.org/jira/browse/HBASE-1873
    Project: Hadoop HBase
    Issue Type: Bug
    Affects Versions: 0.20.0
    Reporter: Eric Tschetter

    HRegionServer on lines 459 - 463 (part of run()) accesses outboundMsgs in a synchronized fashion, but other uses of the object are not synchronized.
    Specifically, the code is
    459 synchronized(this.outboundMsgs) {
    460 outboundArray =
    461 this.outboundMsgs.toArray(new HMsg[outboundMsgs.size()]);
    462 this.outboundMsgs.clear();
    463 }
    Whereas things are added to this list from calls like
    private void reportOpen(HRegionInfo region) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, region));
    }
    And
    void reportSplit(HRegionInfo oldRegion, HRegionInfo newRegionA,
    HRegionInfo newRegionB) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_SPLIT, oldRegion,
    ("Daughters; " +
    newRegionA.getRegionNameAsString() + ", " +
    newRegionB.getRegionNameAsString()).getBytes()));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionA));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionB));
    }
    It looks like the object is initialized as
    private final List<HMsg> outboundMsgs =
    Collections.synchronizedList(new ArrayList<HMsg>());
    Which would appear to provide security, but it doesn't actually prevent an insert from happening between lines 461 and 462, which would subsequently get removed from the call to clear(). At least, from the Sun HotSpot source code, it looks like Collections.synchronizedList() does the right thing and synchronizes on an inner mutex object instead of synchronizing on the externally visible list object itself. That means, however, that the synchronized() on line 459 is largely meaningless.
    I'm not sure how often this race condition would occur in the wild, but every thread waiting on the mutex around the toArray() call increases the probability that the next person to get the mutex is someone who wants to add something to the List, rather than the thread calling clear().
    Simple fix would be to do external synchronization around all accesses to the List. Barring that, perhaps a SynchronizedList implementation with a "emptyToArray()" method that encapsulates the toArray() and subsequent clear().
    --
    This message is automatically generated by JIRA.
    -
    You can reply to this email to add a comment to the issue online.
  • Eric Tschetter (JIRA) at Sep 29, 2009 at 2:02 am
    [ https://issues.apache.org/jira/browse/HBASE-1873?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

    Eric Tschetter updated HBASE-1873:
    ----------------------------------

    Description:
    HRegionServer on lines 459 - 463 (part of run()) accesses outboundMsgs in a synchronized fashion, but other uses of the object are not synchronized.

    Specifically, the code is

    {noformat}
    459 synchronized(this.outboundMsgs) {
    460 outboundArray =
    461 this.outboundMsgs.toArray(new HMsg[outboundMsgs.size()]);
    462 this.outboundMsgs.clear();
    463 }
    {noformat}

    Whereas things are added to this list from calls like

    {noformat}
    private void reportOpen(HRegionInfo region) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, region));
    }
    {noformat}

    And

    {noformat}
    void reportSplit(HRegionInfo oldRegion, HRegionInfo newRegionA,
    HRegionInfo newRegionB) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_SPLIT, oldRegion,
    ("Daughters; " +
    newRegionA.getRegionNameAsString() + ", " +
    newRegionB.getRegionNameAsString()).getBytes()));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionA));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionB));
    }
    {noformat}

    It looks like the object is initialized as

    {noformat}
    private final List<HMsg> outboundMsgs =
    Collections.synchronizedList(new ArrayList<HMsg>());
    {noformat}

    Which would appear to provide security, but it doesn't actually prevent an insert from happening between lines 461 and 462, which would subsequently get removed from the call to clear(). At least, from the Sun HotSpot source code, it looks like Collections.synchronizedList() does the right thing and synchronizes on an inner mutex object instead of synchronizing on the externally visible list object itself. That means, however, that the synchronized() on line 459 is largely meaningless.

    I'm not sure how often this race condition would occur in the wild, but every thread waiting on the mutex around the toArray() call increases the probability that the next person to get the mutex is someone who wants to add something to the List, rather than the thread calling clear().

    Simple fix would be to do external synchronization around all accesses to the List. Barring that, perhaps a SynchronizedList implementation with a "emptyToArray()" method that encapsulates the toArray() and subsequent clear().

    was:
    HRegionServer on lines 459 - 463 (part of run()) accesses outboundMsgs in a synchronized fashion, but other uses of the object are not synchronized.

    Specifically, the code is

    459 synchronized(this.outboundMsgs) {
    460 outboundArray =
    461 this.outboundMsgs.toArray(new HMsg[outboundMsgs.size()]);
    462 this.outboundMsgs.clear();
    463 }

    Whereas things are added to this list from calls like

    private void reportOpen(HRegionInfo region) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, region));
    }

    And

    void reportSplit(HRegionInfo oldRegion, HRegionInfo newRegionA,
    HRegionInfo newRegionB) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_SPLIT, oldRegion,
    ("Daughters; " +
    newRegionA.getRegionNameAsString() + ", " +
    newRegionB.getRegionNameAsString()).getBytes()));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionA));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionB));
    }

    It looks like the object is initialized as

    private final List<HMsg> outboundMsgs =
    Collections.synchronizedList(new ArrayList<HMsg>());

    Which would appear to provide security, but it doesn't actually prevent an insert from happening between lines 461 and 462, which would subsequently get removed from the call to clear(). At least, from the Sun HotSpot source code, it looks like Collections.synchronizedList() does the right thing and synchronizes on an inner mutex object instead of synchronizing on the externally visible list object itself. That means, however, that the synchronized() on line 459 is largely meaningless.

    I'm not sure how often this race condition would occur in the wild, but every thread waiting on the mutex around the toArray() call increases the probability that the next person to get the mutex is someone who wants to add something to the List, rather than the thread calling clear().

    Simple fix would be to do external synchronization around all accesses to the List. Barring that, perhaps a SynchronizedList implementation with a "emptyToArray()" method that encapsulates the toArray() and subsequent clear().

    Race condition around HRegionServer -> HMaster communication
    ------------------------------------------------------------

    Key: HBASE-1873
    URL: https://issues.apache.org/jira/browse/HBASE-1873
    Project: Hadoop HBase
    Issue Type: Bug
    Affects Versions: 0.20.0
    Reporter: Eric Tschetter

    HRegionServer on lines 459 - 463 (part of run()) accesses outboundMsgs in a synchronized fashion, but other uses of the object are not synchronized.
    Specifically, the code is
    {noformat}
    459 synchronized(this.outboundMsgs) {
    460 outboundArray =
    461 this.outboundMsgs.toArray(new HMsg[outboundMsgs.size()]);
    462 this.outboundMsgs.clear();
    463 }
    {noformat}
    Whereas things are added to this list from calls like
    {noformat}
    private void reportOpen(HRegionInfo region) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, region));
    }
    {noformat}
    And
    {noformat}
    void reportSplit(HRegionInfo oldRegion, HRegionInfo newRegionA,
    HRegionInfo newRegionB) {
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_SPLIT, oldRegion,
    ("Daughters; " +
    newRegionA.getRegionNameAsString() + ", " +
    newRegionB.getRegionNameAsString()).getBytes()));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionA));
    outboundMsgs.add(new HMsg(HMsg.Type.MSG_REPORT_OPEN, newRegionB));
    }
    {noformat}
    It looks like the object is initialized as
    {noformat}
    private final List<HMsg> outboundMsgs =
    Collections.synchronizedList(new ArrayList<HMsg>());
    {noformat}
    Which would appear to provide security, but it doesn't actually prevent an insert from happening between lines 461 and 462, which would subsequently get removed from the call to clear(). At least, from the Sun HotSpot source code, it looks like Collections.synchronizedList() does the right thing and synchronizes on an inner mutex object instead of synchronizing on the externally visible list object itself. That means, however, that the synchronized() on line 459 is largely meaningless.
    I'm not sure how often this race condition would occur in the wild, but every thread waiting on the mutex around the toArray() call increases the probability that the next person to get the mutex is someone who wants to add something to the List, rather than the thread calling clear().
    Simple fix would be to do external synchronization around all accesses to the List. Barring that, perhaps a SynchronizedList implementation with a "emptyToArray()" method that encapsulates the toArray() and subsequent clear().
    --
    This message is automatically generated by JIRA.
    -
    You can reply to this email to add a comment to the issue online.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupdev @
categorieshbase, hadoop
postedSep 29, '09 at 1:58a
activeSep 29, '09 at 2:02a
posts3
users1
websitehbase.apache.org

1 user in discussion

Eric Tschetter (JIRA): 3 posts

People

Translate

site design / logo © 2021 Grokbase