You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2008/10/19 13:50:55 UTC
Re: svn commit: r33750 - trunk/subversion/libsvn_wc
Good catch! Thanks for the tweak.
On Sun, Oct 19, 2008 at 5:53 AM, <rh...@tigris.org> wrote:
> Author: rhuijben
> Date: Sun Oct 19 05:53:39 2008
> New Revision: 33750
>
> Log:
> * subversion/libsvn_wc/lock.c
> Following up on r33676, skip writing to the now read only missing instance.
> This should fix several testcases on the Windows buildbot.
>
> Modified:
> trunk/subversion/libsvn_wc/lock.c
>
> Modified: trunk/subversion/libsvn_wc/lock.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/lock.c?pathrev=33750&r1=33749&r2=33750
> ==============================================================================
> --- trunk/subversion/libsvn_wc/lock.c Sat Oct 18 18:59:17 2008 (r33749)
> +++ trunk/subversion/libsvn_wc/lock.c Sun Oct 19 05:53:39 2008 (r33750)
> @@ -758,6 +758,13 @@ do_open(svn_wc_adm_access_t **adm_access
> apr_hash_this(hi, &key, NULL, &val);
> entry_path = key;
> entry_access = val;
> +
> + if (entry_access == &missing)
> + {
> + /* Entry is missing or obstructed; see above */
> + continue; /* Skip or we will write read only memory */
> + }
> +
> apr_hash_set(shared->set, entry_path, APR_HASH_KEY_STRING,
> entry_access);
> entry_access->shared = shared;
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33750 - trunk/subversion/libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
&missing is only ever placed into hash tables, and all of the hash
table gets and iterations always check for missing. So I dunno how it
might get written into.
Shouldn't we be able to get a stack trace off the Windows box? That
would point us in the right direction immediately.
Cheers,
-g
On Sun, Oct 19, 2008 at 9:21 AM, Greg Stein <gs...@gmail.com> wrote:
> Yeah, I'll take a look into it. As you noted on IRC, I'm surprised
> that Linux doesn't put this stuff into a readonly data segment, and
> that people would then see it. Personally, I'm on a Mac and I dunno
> much about its linker/loader. It certainly didn't have a problem.
>
> And since the tests *did* see it, then we're getting coverage of that section.
>
> Anyways... I'll review the code and see what I can find. It's possible
> that code I haven't changed/looked-at does an incorrect write. And of
> course, maybe something that I *did* change :-)
>
> Cheers,
> -g
>
> On Sun, Oct 19, 2008 at 8:59 AM, Bert Huijben <be...@vmoo.com> wrote:
>>> -----Original Message-----
>>> From: Greg Stein [mailto:gstein@gmail.com]
>>> Sent: Sunday, October 19, 2008 3:51 PM
>>> To: dev@subversion.tigris.org; rhuijben@tigris.org
>>> Subject: Re: svn commit: r33750 - trunk/subversion/libsvn_wc
>>>
>>> Good catch! Thanks for the tweak.
>>
>> Greg,
>>
>> This fix and my follow up don't seem to fix the problem that some other code
>> depends on the global static struct is writable.
>>
>> Can you take a look at this?
>>
>> The crash Sander reported is that Visual C++ put the static const structure
>> in a read only memory segment; the other compilers/OSs seem to allow writing
>> to it. I don't think marking the structure non-const is a real solution, as
>> this will give multithreading issues.
>>
>> Thanks,
>>
>> Bert
>>
>>>
>>> On Sun, Oct 19, 2008 at 5:53 AM, <rh...@tigris.org> wrote:
>>> > Author: rhuijben
>>> > Date: Sun Oct 19 05:53:39 2008
>>> > New Revision: 33750
>>> >
>>> > Log:
>>> > * subversion/libsvn_wc/lock.c
>>> > Following up on r33676, skip writing to the now read only missing
>>> instance.
>>> > This should fix several testcases on the Windows buildbot.
>>> >
>>> > Modified:
>>> > trunk/subversion/libsvn_wc/lock.c
>>> >
>>> > Modified: trunk/subversion/libsvn_wc/lock.c
>>> > URL:
>>> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/lock.c?path
>>> rev=33750&r1=33749&r2=33750
>>> >
>>> =======================================================================
>>> =======
>>> > --- trunk/subversion/libsvn_wc/lock.c Sat Oct 18 18:59:17 2008
>>> (r33749)
>>> > +++ trunk/subversion/libsvn_wc/lock.c Sun Oct 19 05:53:39 2008
>>> (r33750)
>>> > @@ -758,6 +758,13 @@ do_open(svn_wc_adm_access_t **adm_access
>>> > apr_hash_this(hi, &key, NULL, &val);
>>> > entry_path = key;
>>> > entry_access = val;
>>> > +
>>> > + if (entry_access == &missing)
>>> > + {
>>> > + /* Entry is missing or obstructed; see above */
>>> > + continue; /* Skip or we will write read only
>>> memory */
>>> > + }
>>> > +
>>> > apr_hash_set(shared->set, entry_path,
>>> APR_HASH_KEY_STRING,
>>> > entry_access);
>>> > entry_access->shared = shared;
>>> >
>>> > ---------------------------------------------------------------------
>>> > To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
>>> > For additional commands, e-mail: svn-help@subversion.tigris.org
>>> >
>>> >
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33750 - trunk/subversion/libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
Yeah, I'll take a look into it. As you noted on IRC, I'm surprised
that Linux doesn't put this stuff into a readonly data segment, and
that people would then see it. Personally, I'm on a Mac and I dunno
much about its linker/loader. It certainly didn't have a problem.
And since the tests *did* see it, then we're getting coverage of that section.
Anyways... I'll review the code and see what I can find. It's possible
that code I haven't changed/looked-at does an incorrect write. And of
course, maybe something that I *did* change :-)
Cheers,
-g
On Sun, Oct 19, 2008 at 8:59 AM, Bert Huijben <be...@vmoo.com> wrote:
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein@gmail.com]
>> Sent: Sunday, October 19, 2008 3:51 PM
>> To: dev@subversion.tigris.org; rhuijben@tigris.org
>> Subject: Re: svn commit: r33750 - trunk/subversion/libsvn_wc
>>
>> Good catch! Thanks for the tweak.
>
> Greg,
>
> This fix and my follow up don't seem to fix the problem that some other code
> depends on the global static struct is writable.
>
> Can you take a look at this?
>
> The crash Sander reported is that Visual C++ put the static const structure
> in a read only memory segment; the other compilers/OSs seem to allow writing
> to it. I don't think marking the structure non-const is a real solution, as
> this will give multithreading issues.
>
> Thanks,
>
> Bert
>
>>
>> On Sun, Oct 19, 2008 at 5:53 AM, <rh...@tigris.org> wrote:
>> > Author: rhuijben
>> > Date: Sun Oct 19 05:53:39 2008
>> > New Revision: 33750
>> >
>> > Log:
>> > * subversion/libsvn_wc/lock.c
>> > Following up on r33676, skip writing to the now read only missing
>> instance.
>> > This should fix several testcases on the Windows buildbot.
>> >
>> > Modified:
>> > trunk/subversion/libsvn_wc/lock.c
>> >
>> > Modified: trunk/subversion/libsvn_wc/lock.c
>> > URL:
>> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/lock.c?path
>> rev=33750&r1=33749&r2=33750
>> >
>> =======================================================================
>> =======
>> > --- trunk/subversion/libsvn_wc/lock.c Sat Oct 18 18:59:17 2008
>> (r33749)
>> > +++ trunk/subversion/libsvn_wc/lock.c Sun Oct 19 05:53:39 2008
>> (r33750)
>> > @@ -758,6 +758,13 @@ do_open(svn_wc_adm_access_t **adm_access
>> > apr_hash_this(hi, &key, NULL, &val);
>> > entry_path = key;
>> > entry_access = val;
>> > +
>> > + if (entry_access == &missing)
>> > + {
>> > + /* Entry is missing or obstructed; see above */
>> > + continue; /* Skip or we will write read only
>> memory */
>> > + }
>> > +
>> > apr_hash_set(shared->set, entry_path,
>> APR_HASH_KEY_STRING,
>> > entry_access);
>> > entry_access->shared = shared;
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
>> > For additional commands, e-mail: svn-help@subversion.tigris.org
>> >
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: svn commit: r33750 - trunk/subversion/libsvn_wc
Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: Sunday, October 19, 2008 3:51 PM
> To: dev@subversion.tigris.org; rhuijben@tigris.org
> Subject: Re: svn commit: r33750 - trunk/subversion/libsvn_wc
>
> Good catch! Thanks for the tweak.
Greg,
This fix and my follow up don't seem to fix the problem that some other code
depends on the global static struct is writable.
Can you take a look at this?
The crash Sander reported is that Visual C++ put the static const structure
in a read only memory segment; the other compilers/OSs seem to allow writing
to it. I don't think marking the structure non-const is a real solution, as
this will give multithreading issues.
Thanks,
Bert
>
> On Sun, Oct 19, 2008 at 5:53 AM, <rh...@tigris.org> wrote:
> > Author: rhuijben
> > Date: Sun Oct 19 05:53:39 2008
> > New Revision: 33750
> >
> > Log:
> > * subversion/libsvn_wc/lock.c
> > Following up on r33676, skip writing to the now read only missing
> instance.
> > This should fix several testcases on the Windows buildbot.
> >
> > Modified:
> > trunk/subversion/libsvn_wc/lock.c
> >
> > Modified: trunk/subversion/libsvn_wc/lock.c
> > URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/lock.c?path
> rev=33750&r1=33749&r2=33750
> >
> =======================================================================
> =======
> > --- trunk/subversion/libsvn_wc/lock.c Sat Oct 18 18:59:17 2008
> (r33749)
> > +++ trunk/subversion/libsvn_wc/lock.c Sun Oct 19 05:53:39 2008
> (r33750)
> > @@ -758,6 +758,13 @@ do_open(svn_wc_adm_access_t **adm_access
> > apr_hash_this(hi, &key, NULL, &val);
> > entry_path = key;
> > entry_access = val;
> > +
> > + if (entry_access == &missing)
> > + {
> > + /* Entry is missing or obstructed; see above */
> > + continue; /* Skip or we will write read only
> memory */
> > + }
> > +
> > apr_hash_set(shared->set, entry_path,
> APR_HASH_KEY_STRING,
> > entry_access);
> > entry_access->shared = shared;
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: svn-help@subversion.tigris.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org