Bug report: Data race in pcache1.c

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

Bug report: Data race in pcache1.c

Jan Krčál
Hi,

as a Chrome developer, I've run into a data race flagged by ThreadSanitizer
in pcache1.c:

When two threads write to in-memory sqlite DBs (each to its own separate
DB), each thread has its own PCache1 with bPurgeable being false. Thus, in
both threads, pCache->pnPurgeable points to the static
variable dummyCurrentPage.

Even if bPurgeable is false, pCache->pnPurgeable gets incremented by
pcache1AllocPage()  and decremented by pcache1FreePage(). When each pCache
belongs to a different pGroup, the writes can happen concurrently on those
two threads and are not protected by any mutex. This results in a data race
that makes ThreadSanitizer unhappy.

These writes are not needed in the first place because the value of
pCache->pnPurgeable is never read if bPurgeable is false. Because the value
is never read, I can hardly imagine a situation this data race could lead
to a real problem. Still, it is technically a data race with undefined
behavior. Most importantly, I'd like ThreadSanitizer be happy with this
code in this context.

This is the first time I look into sqlite source code, does the text above
make sense to you? :)
If so, the fix should be pretty simple. Can you please look into this or
should I provide a patch?

Best regards,
Jan
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Bug report: Data race in pcache1.c

Richard Hipp-3
Check-in https://www.sqlite.org/src/info/383437be276719ac will perhaps
silence the harmless false-positives reported by TSAN.  Please let us
know if it does not solve the problem for you.

On 1/7/19, Jan Krčál <[hidden email]> wrote:

> Hi,
>
> as a Chrome developer, I've run into a data race flagged by ThreadSanitizer
> in pcache1.c:
>
> When two threads write to in-memory sqlite DBs (each to its own separate
> DB), each thread has its own PCache1 with bPurgeable being false. Thus, in
> both threads, pCache->pnPurgeable points to the static
> variable dummyCurrentPage.
>
> Even if bPurgeable is false, pCache->pnPurgeable gets incremented by
> pcache1AllocPage()  and decremented by pcache1FreePage(). When each pCache
> belongs to a different pGroup, the writes can happen concurrently on those
> two threads and are not protected by any mutex. This results in a data race
> that makes ThreadSanitizer unhappy.
>
> These writes are not needed in the first place because the value of
> pCache->pnPurgeable is never read if bPurgeable is false. Because the value
> is never read, I can hardly imagine a situation this data race could lead
> to a real problem. Still, it is technically a data race with undefined
> behavior. Most importantly, I'd like ThreadSanitizer be happy with this
> code in this context.
>
> This is the first time I look into sqlite source code, does the text above
> make sense to you? :)
> If so, the fix should be pretty simple. Can you please look into this or
> should I provide a patch?
>
> Best regards,
> Jan
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>


--
D. Richard Hipp
[hidden email]
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users