From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8410 invoked by alias); 8 Apr 2003 16:16:01 -0000 Mailing-List: contact gcc-prs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-prs-owner@gcc.gnu.org Received: (qmail 8391 invoked by uid 71); 8 Apr 2003 16:16:01 -0000 Resent-Date: 8 Apr 2003 16:16:01 -0000 Resent-Message-ID: <20030408161601.8390.qmail@sources.redhat.com> Resent-From: gcc-gnats@gcc.gnu.org (GNATS Filer) Resent-Cc: gcc-prs@gcc.gnu.org, gcc-bugs@gcc.gnu.org Resent-Reply-To: gcc-gnats@gcc.gnu.org, jkanze@caicheuvreuse.com Received: (qmail 5484 invoked by uid 48); 8 Apr 2003 16:12:21 -0000 Message-Id: <20030408161221.5483.qmail@sources.redhat.com> Date: Tue, 08 Apr 2003 16:16:00 -0000 From: jkanze@caicheuvreuse.com Reply-To: jkanze@caicheuvreuse.com To: gcc-gnats@gcc.gnu.org X-Send-Pr-Version: gnatsweb-2.9.3 (1.1.1.1.2.31) Subject: libstdc++/10350: thread-safety problem in std::string. X-SW-Source: 2003-04/txt/msg00346.txt.bz2 List-Id: >Number: 10350 >Category: libstdc++ >Synopsis: thread-safety problem in std::string. >Confidential: no >Severity: serious >Priority: medium >Responsible: unassigned >State: open >Class: sw-bug >Submitter-Id: net >Arrival-Date: Tue Apr 08 16:16:01 UTC 2003 >Closed-Date: >Last-Modified: >Originator: James Kanze >Release: g++ 3.2.2 >Organization: >Environment: >Description: I believe I've found an error in the thread management in std::string in the library. Basically, the problem occurs in the following context: - There is a global variable declared: std::string s ; This variable is initialised by reading from a configuration file, in main, before any threads have been initialized. It is non-const so that this initialization can occur; it is never modified afterwards. Since it is never modified once threads have been started, no accesses are protected by mutex's or anything else. - A copy of this variable is made to another string variable. This causes the reference count _M_references in basic_string::_Rep to become 1, with the result that _Rep::_M_is_shared will return true. - Several threads are started. At exactly the same moment, in thread A and thread B, an expression along the lines of "s[ i ] == 'x'" is executed. Note that because the instance is not const, the function called is the non-const version of basic_string::operator[], even though nobody is going to modify anything. (And because nobody is modifying the string in any of the threads, there are no locks.) This function begins by calling _M_leak; since _Rep isn't yet leaked, this function calls _M_leak_hard, and since the _Rep is shared, we end up in _M_mutate. _M_mutate, after calculating a lot of different sizes (which aren't really relevant here), executes an if, which evaluates true. The last two lines of the if are the critical section: imagine that thread A has been executing up until now, and it is interrupted by thread B. At this point, thread A has not modified anything in the actual object -- it has made a series of tests, decided that it must isolate the representation, and has prepared a new representation, but it has not started installing it. Thread B executes the same code, and since the actual object has not been changed, comes to the same conclusions, and arrives at the same point. From this point, several scenarios are possible: thread B continues, calling dispose on the _Rep in the actual object (which, since there is still another object using it, will not be destructed), installs its own newly created _Rep and returns a reference to the data in it. At this point, thread A takes over again, calls _M_rep()->_M_dispose, which disposes of the _Rep thread B has just installed -- since the just installed _Rep is not shared it is deleted, and the reference in B is invalid. For the moment, I'm not convinced that a copy on write implementation of the std::string interface is possible with only the primitives atomic_incr, atomic_decr, barrier_read and barrier_write -- the current g++ implementation tries to do it with even less. (Note that the current implementation fails lamentably in a multi-processor environment; there is no effort what so ever made to ensure that writes are actually seen by the other processes. But I've not much experience in evaluating this sort of problem.) Might I suggest that the current implementation be completely scrapped, in favor of a simple deep copy. The results will be somewhat slower, but at least they will work. Alternatively, if you have some mechanism of switching implementations depending on the -mt switch of the compiler... (I've found COW to be a definite win in my single threaded applications.) >How-To-Repeat: >Fix: >Release-Note: >Audit-Trail: >Unformatted: