Bounds exception when using deepcopy with regex

[tjgreen42]

julia> match(deepcopy(r""), "")
ERROR: BoundsError: attempt to access ""
  at index [1829569203750929]
Stacktrace:
 [1] prevind(::String, ::Int64, ::Int64) at ./strings/basic.jl:460
 [2] prevind at ./strings/basic.jl:455 [inlined]
 [3] prevind at ./strings/basic.jl:454 [inlined]
 [4] match(::Regex, ::String, ::Int64, ::UInt32) at ./regex.jl:214
 [5] match at ./regex.jl:207 [inlined]
 [6] match(::Regex, ::String) at ./regex.jl:222
 [7] top-level scope at none:0```

As discussed on Slack with @StefanKarpinski and @vtjnash, the right fix is probably to provide an overload of deepcopy_internal for Regex which makes a copy of the underlying PCRE object, ideally through the PCRE API if such functionality exists.

[tjgreen42]

While inspecting the Regex/PCRE code, I also noticed that the call to compile inside match is probably unnecessary, since the Regex inner constructor already calls compile.

[JeffBezanson]

Is it actually meaningful to copy a regex, or could we make deepcopy_internal just return the original object?

[tjgreen42]

@JeffBezanson Dunno, what about multithreading, I can imagine people might be tempted to use deepcopy to create independent copies of an object to avoid concurrency issues?

[StefanKarpinski]

Is it actually meaningful to copy a regex, or could we make deepcopy_internal just return the original object?

This seems like it would be fine and the simplest solution to this, although it doesn't address the multithreading issue. But that issue would still exist anyway since multiple threads could have references to the same uncopied regex object, which means that if we want to make regexes threadsafe, we'd either need to put a mutex around the use of match_data or make it per-thread.

[Moelf]

Is this already fixed?

julia> match(deepcopy(r""), "")
RegexMatch("")

[eulerkochy]

Well, I was experimenting for this issue, and I found out for Regex, out of the 7 fields, one of them is an Array, which implies deepcopy(r"") === r"" will always be false. The simplest way out as @JeffBezanson suggested would be to make a separate deepcopy_internal for Regex type and avoid this issue. That being said, it might happen this might be one of the several types that are facing this problem, (basically any type with Array as one their field). What's the way out in those cases? We can't hardcode everytime, right?

[vtjnash]

Is it actually meaningful to copy a regex

Depends on what you think "deepcopy" should mean. There's bits of internal state (mostly write-once/immutable), and an allocation cache (mutable) inside. But otoh, there's no copy method since it doesn't expose a mutation API (although some mutation does happen, as a side-effect of usage).

[vtjnash]

That being said, it might happen this might be one of the several types that are facing this problem, (basically any type with Array as one their field)

I think the issue is often with types that have a Ptr field and/or a finalizer (often those go together)—and Regex has both. This issue has also been previously discussed in #16667.